From 04aa7971d1cab1256b97756859f3f556082231d5 Mon Sep 17 00:00:00 2001 From: graphlemon Date: Sat, 8 Aug 2026 11:37:55 +0800 Subject: [PATCH] graphlemon: auto-commit uncommitted changes for 18a9005c-aba8-4e35-88a6-184de4b94843-main-checkout-drift --- docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md | 37 +++++++++++++++++++++++++++++++++++++ docs/todo/validator-test-script-narrative-output-issue.md | 16 +++++++++++++++- docs/todo/work-graph-self-planning-and-loop-back-design.md | 643 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ docs/todo/work-graph-self-planning-and-loop-back-requirements.md | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/backend_server.py | 2 ++ src/e2e_pipeline.py | 65 +++++++++++++++++++++++++++++++++++++++++++++++++---------------- src/schema.py | 69 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/settings_config.py | 38 ++++++++++++++++++++++++++++++++++++++ src/test_e2e_pipeline.py | 199 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------ src/test_schema.py | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++-- src/test_settings_config.py | 37 +++++++++++++++++++++++++++++++++++++ src/test_work_graph_orchestrate.py | 535 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ src/test_work_graph_plan.py | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++------------------------------------------------- src/work_graph_orchestrate.py | 540 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---------------------------------------- src/work_graph_plan.py | 245 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--------------------------------------------------------- 15 files changed, 2633 insertions(+), 203 deletions(-) create mode 100644 docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md create mode 100644 docs/todo/work-graph-self-planning-and-loop-back-design.md create mode 100644 docs/todo/work-graph-self-planning-and-loop-back-requirements.md diff --git a/docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md b/docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md new file mode 100644 index 0000000..ba15035 --- /dev/null +++ b/docs/todo/task-execution-resume-assumes-planner-artifacts-exist.md @@ -0,0 +1,37 @@ +# `task_execution.py` 恢复逻辑假设 Planner 一定已产出 sdd/prd/benchmark artifact——问题记录 + +**日期**: 2026-08-08 +**性质**: 问题记录（真实复现，根因已定位，未修复）。发现于 `docs/todo/work-graph-self-planning-and-loop-back-requirements.md`（Planner 自规划）上线后的第一次真实端到端验证。 +**状态**: 待修复，未立项。 + +## 1. 现象（真实复现） + +任务 `2084590a`（目标：修复 `work_node_run.py` 的 `execution_log_file` UNIQUE 约束 bug）： + +1. 第一次真实调用：Planner 自规划出 `sdd → benchmark → test_script` 三节点图（没有 brd/prd——goal 是个小 bug 修复，Planner 判断不需要），`sdd` 节点的 `build_prompt()` 因为 `_DOC_PROMPTS["sdd"]` 硬编码的 `{prd_content}` 占位符找不到值而抛 `KeyError('prd_content')`（已修复，见 `e2e_pipeline.py::_MissingContentDefault`）。此时 `sdd` 节点本身从未成功过，`artifact` 表里这个 task 一行都没有。 +2. 通过 `/route` 的 `manual_resume` 恢复：`task.status` 被设置回 `planning_brd_prd_sdd`。 +3. 重新执行时走的是 `task_execution.py::run_or_resume_task_lifecycle()` 的"从 `planning_brd_prd_sdd` 恢复"分支，而不是从头重跑 `plan_documents()`——这个分支无条件调用 `_artifact_content(conn, ..., "sdd")`/`"prd"`/`"benchmark"`，读不到就直接 `TaskExecutionError`。因为这个 task 的 Planner 图从未成功过一个节点，三个 artifact 全部不存在，报错为 `"task ... has no sdd artifact to resume from"`，任务再次卡住。 + +## 2. 根因 + +`org_node_lifecycle.trigger_org_node_full(conn, "planner", ...)` 在 `run_task_lifecycle()` 里**先于** `plan_documents()` 执行，并把 `task.status` 设成 `_POST_STATE["planner"] = "planning_brd_prd_sdd"`（`org_node_lifecycle.py:32`）——这个状态转移只代表"Planner 的 ready 确认调用成功了"，不代表"Planner 的真实规划（`plan_documents()`）已经产出了任何东西"。 + +`task_execution.py::run_or_resume_task_lifecycle()` 里 `planning_brd_prd_sdd` 分支（`task_execution.py:126-129`）的注释写的是"Planner done, Coder not yet triggered"——这个假设在 Work Graph 自规划（`docs/todo/work-graph-self-planning-and-loop-back-requirements.md`）上线前基本成立：固定链的第一个节点是 brd，几乎不可能在第一个节点就失败到零产出。自规划之后这个假设不再可靠——自规划出的图可能把一个更容易失败的节点（比如需要更复杂 judgment 的 sdd）排在第一位,如果它在第一次尝试就彻底失败（耗尽 loop_back/retry 上限,或者像这次一样是一个跟自规划本身相关的新 bug),Planner 的 `work_graph_run` 会整个失败,不会有任何 artifact 落盘,但 `task.status` 已经先一步被置成了 `planning_brd_prd_sdd`。 + +`test_dataset_content`/`test_script_content` 已经用 `_artifact_content_or_default()`（读不到就给默认值,不硬失败）处理过同一类问题（2026-08-06,那次是"这两个 artifact 类型本来就允许不存在"），但 `sdd`/`prd`/`benchmark` 三个仍然用会硬失败的 `_artifact_content()`——这个区分在固定链时代是对的（sdd/prd/benchmark 曾经是保证存在的),自规划之后不再成立。 + +## 3. 跟已实现的两个相关修复的区别 + +- 不是 `work-graph-partial-artifact-persistence-requirements.md` 那次修复能覆盖的场景——那次解决的是"图跑到一半、后面节点失败,前面成功节点的产出要保留",这次是"图的第一个节点就彻底失败,压根没有'前面成功节点'可保留"，`_write_completed_node_artifacts` 在这种情况下正确地什么都不落盘（因为真的什么都没成功),不是它的 bug。 +- 不是这次 session 里刚修的 `_MissingContentDefault`（`e2e_pipeline.py::_planner_prompt_builder`）能覆盖的——那个修的是"build_prompt 拼提示词时崩溃"，这次是崩溃之后**恢复逻辑**本身的假设过时了，是下游的另一层问题。 + +## 4. 可能的修复方向（未验证，仅供后续参考） + +在 `run_or_resume_task_lifecycle()` 的 `planning_brd_prd_sdd` 分支里，先检查这个 task 是否**真的有任何 doc 类 artifact**（`_latest_artifact_ids(conn, task_id, _DOC_ARTIFACT_TYPES)` 是否为空）： + +- 如果有（哪怕只有部分,比如这次自规划场景下可能只有 sdd 没有 prd/benchmark),按现有逻辑继续,但 `prd`/`benchmark` 也应该跟 `test_dataset`/`test_script` 一样改用 `_artifact_content_or_default`,不再假设它们一定存在（自规划场景下 prd/benchmark 本来就可能被 Planner 判断为不需要,不是"缺失的异常情况"）。 +- 如果一个都没有（这次真实复现的情况——Planner 的图从第一个节点就彻底失败,什么都没留下),说明"从这个状态恢复"这个假设本身不成立,应该等价于从 `queued` 重新完整跑一次 `run_task_lifecycle()`（即重新调用 `plan_documents()`),而不是尝试读取不存在的 artifact。 + +## 5. 临时规避（已用于解决这次真实卡住的任务） + +直接把 `task.status` 手动改回 `queued`（`UPDATE task SET status='queued', blocked_reason=NULL WHERE task_id=...`),让下一次执行走 `run_task_lifecycle()` 的"queued"分支、重新完整调用一次 `plan_documents()`——不是通用修复,只是这一次真实卡住的任务的人工解法。 diff --git a/docs/todo/validator-test-script-narrative-output-issue.md b/docs/todo/validator-test-script-narrative-output-issue.md index a0f12df..33fcba8 100644 --- a/docs/todo/validator-test-script-narrative-output-issue.md +++ b/docs/todo/validator-test-script-narrative-output-issue.md @@ -2,7 +2,7 @@ **日期**: 2026-08-07 **性质**: 问题记录 + 待调查,不是已确认根因的需求文档（区别于同目录下其他几份——那几份是已经定位到具体代码 bug 并修复过的,这份目前还只是"观察到的现象",根因未确定）。 -**状态**: 根因已定位（见第 6 节），低成本缓解已实现（见第 7 节，`_NARRATION_RETRY_HINT`）。仍待验证：下次真实自举任务的 test_script 节点是否还会复现——这个缓解只改了重试反馈的措辞，不是结构性修复，第一次生成仍可能叙述，只是预期重试成功率会提高。**2026-08-07 更新（第二轮）：不只是 Validator 的问题——`911861fe` 这个纯后端任务在 Planner 自己的 `n_test_script` 节点上也复现了同一模式。累计 3 个任务、6 次到达 test_script 类节点，6 次全部复现（100%），横跨 Planner 和 Validator 两种场景。建议提高优先级，不要再靠重试撞运气。** **2026-08-07 更新（第三轮，根因排查）：这是 08-06 "Benchmark 叙述事故"（commit `07a0a27`）的同类复发——test_script 类节点当时已经同时拿到提示词纠偏和机械拦截网双重防护，仍然 6/6 复现。根因不是"self-hosting 词汇污染"这个泛泛的猜测，而是 test_script 提示词本身要求"先做工具探索再产出内容"，模型探索后的收尾话术天然是"汇报我做了/启动了什么"而不是切换成纯文本吐出模式；self-hosting 只是放大器，让叙述读起来更精确（因为探索对象就是 graphlemon 自己，词汇表精确对应），不是必要条件。BRD/PRD/SDD/Benchmark/test_dataset 这几个不需要先探索的纯组合型节点，自 08-06 后再未复现，与此吻合。** +**状态**: 根因已定位（见第 6 节）。用户决策（2026-08-07）：不再继续在提示词/重试反馈上打补丁，**先直接禁用 test_script/validator_test_script 节点**（Planner 的 `work_graph_plan._PROMPT_TEMPLATE` 不再要求这个节点，Validator 的 `e2e_pipeline._VALIDATOR_MANIFEST` 从 3 节点缩成 2 节点），后续如果真的需要再重新设计（见第 8 节的结构性修复方向：把"探索"和"最终作答"拆成两次独立调用）。`_NARRATION_RETRY_HINT`（第 7 节）代码仍保留在 `_test_script_verify_fn` 里,只是当前这条路径不会被触发——如果以后重新启用这个节点,这个缓解依然生效。**2026-08-08 更新（第四轮，与自规划 Work Graph 需求的交叉决策）**：`docs/todo/work-graph-self-planning-and-loop-back-requirements.md`（Planner 自规划节点集合 + 失败回路需求）上线后，test_script 会重新作为可选节点开放给 Planner 自由规划——用户明确决定**不等本文档的结构性修复**，narrative-output 问题可能因此重现，这是已知且被接受的风险。本文档记录的根因（第 6 节）的真正结构性修复，安排在**另一份尚未撰写、关于"给 Coder agent 加入 test_script 模块"的文档**里一起解决，不是本文档单独继续跟进的范围。**2026-08-07 更新（第二轮）：不只是 Validator 的问题——`911861fe` 这个纯后端任务在 Planner 自己的 `n_test_script` 节点上也复现了同一模式。累计 3 个任务、6 次到达 test_script 类节点，6 次全部复现（100%），横跨 Planner 和 Validator 两种场景。建议提高优先级，不要再靠重试撞运气。** **2026-08-07 更新（第三轮，根因排查）：这是 08-06 "Benchmark 叙述事故"（commit `07a0a27`）的同类复发——test_script 类节点当时已经同时拿到提示词纠偏和机械拦截网双重防护，仍然 6/6 复现。根因不是"self-hosting 词汇污染"这个泛泛的猜测，而是 test_script 提示词本身要求"先做工具探索再产出内容"，模型探索后的收尾话术天然是"汇报我做了/启动了什么"而不是切换成纯文本吐出模式；self-hosting 只是放大器，让叙述读起来更精确（因为探索对象就是 graphlemon 自己，词汇表精确对应），不是必要条件。BRD/PRD/SDD/Benchmark/test_dataset 这几个不需要先探索的纯组合型节点，自 08-06 后再未复现，与此吻合。** ## 1. 现象 @@ -79,3 +79,17 @@ self-hosting 场景（`target_folder_path` 是 graphlemon 自己）在这个机 设计上刻意用条件句（"If your previous output was..."）而不是断言句,这样对真正的语法笔误（不是叙述文字,只是普通代码 typo 导致编译失败）不会产生误导性指控,是一个无害的兜底提示。`_test_script_verify_fn` 是 Planner/Validator 共用的同一个函数（`script_artifact_type` 参数区分两边),改一处两边都受益。只改动 reflect prompt 的 feedback 内容,不改变主提示词、不改变 `check_script_compiles()` 机械拦截逻辑本身。 后续如果这个缓解仍不够（比如下次真实自举任务再次复现),下一步该看第 6.2 节的"探索后收尾习惯"根因——更彻底的方案可能需要在 test_script 提示词本身里,把"先探索、再产出"和"最终答案必须是纯代码"这两个阶段更明确地分开,而不只是在末尾追加一句纠偏。 + +## 8. 已实现（2026-08-07，第二次决策）：直接禁用 test_script/validator_test_script 节点 + +用户决策：与其继续在提示词层面打补丁（第 7 节的缓解已经证明有效性无法在下次真实复现前确认）,不如先把这两个节点从图里直接去掉,后续真的有需求再重新设计,而不是"所有 goal 任务都固定有这个节点"。这也牵出一个更大的架构问题：Planner 自己的文档链（`work_graph_plan._PROMPT_TEMPLATE`）虽然过了一层真实的 LLM 规划调用,但这次调用的提示词硬编码要求"必须为 brd/prd/sdd/benchmark/test_dataset/test_script 各出一个节点"——是一个固定架构常量（2026-08-06 的既有设计决定，见 `docs/done/dynamic-work-graph-design-skill-design.md §3`）,不是真的由 LLM 按 goal 自由决定节点。Validator 自己的验证链（`_VALIDATOR_MANIFEST`）更彻底,连"过一层 LLM 规划"都没有,是硬编码的 Python 字面量。真正"LLM 自己决定要几个节点"的能力已经存在于 `_GENERIC_PROMPT_TEMPLATE`（给 Coder/Validator 其他子任务用),但没有用在这两条文档链上——是否要把这条能力也用到 Planner/Validator 自己的文档链上,是一个更大的、需要单独决策的后续方向,不在本次改动范围内。 + +**具体改动**： +- `src/work_graph_plan.py::_PROMPT_TEMPLATE`：不再要求 test_script 节点（只要求 brd/prd/sdd/benchmark/test_dataset 5 个）。 +- `src/work_graph_plan.py::_CANONICAL_DOC_CHAIN`：从 6 元组缩成 5 元组,同步去掉了原本专门给 test_script 节点注入"额外依赖 benchmark"的那段代码（`_normalize_graph_manifest` 里）。 +- `src/e2e_pipeline.py::_VALIDATOR_MANIFEST`：从 3 节点（validator_benchmark→validator_test_dataset→validator_test_script）缩成 2 节点。 +- `validate_implementation_with_own_graph()`：`outputs["validator_test_script"]` 的访问加了 `in` 判断,不存在时 `script_execution` 走"N/A: 节点已禁用"的兜底分支,verdict 完全由 LLM 自己的判断决定（不再有脚本硬门禁）。 +- 下游消费者（`run_task_lifecycle`/`task_execution.py` 里读 test_script artifact 内容喂给 Coder 的逻辑）本来就已经用 `"test_script" in artifact_ids`/`_artifact_content_or_default` 做了容错（这是 2026-08-06 test_script 刚上线时,为了兼容"这个节点之前不存在的旧任务"而写的防御性代码）,这次改动不需要再碰这两处。 +- `_DOC_PROMPTS["test_script"]`/`_VALIDATOR_DOC_PROMPTS["validator_test_script"]`/`check_script_compiles()`/`_test_script_verify_fn`/`_NARRATION_RETRY_HINT` 等下游代码全部原样保留,只是不会再被触发——如果以后要重新启用,只需要把这两处节点定义加回去。 + +**测试**：`src/test_work_graph_plan.py`/`src/test_e2e_pipeline.py` 里原本依赖 6/3 节点固定结构的用例做了相应调整；其中"晚失败节点不连累早节点产出"（partial-artifact-persistence 的回归用例）和"脚本真失败强制 FAIL"（hard-gate 用例）改成显式用一个临时补丁把 test_script/validator_test_script 节点加回manifest 来测试——因为这两块机制本身仍是真实代码,只是默认不会被规划出来,不应该因为默认禁用就失去回归覆盖。`python3 -m unittest discover -s src`（643 tests）+ `discover -s supervisor -p "test_*.py"`（126 tests）全绿。 diff --git a/docs/todo/work-graph-self-planning-and-loop-back-design.md b/docs/todo/work-graph-self-planning-and-loop-back-design.md new file mode 100644 index 0000000..abf0b2c --- /dev/null +++ b/docs/todo/work-graph-self-planning-and-loop-back-design.md @@ -0,0 +1,643 @@ +# Planner 动态自规划 Work Graph（含节点判断与失败回路）——详细设计（SDD） + +**日期**: 2026-08-08 +**性质**: 详细设计，对应需求文档 `docs/todo/work-graph-self-planning-and-loop-back-requirements.md`（已确认）。本设计给出可直接实施的 schema/接口/算法/测试方案，逐条落实需求文档「8. 已确认的决策」。 +**依赖代码**：`src/work_graph_orchestrate.py`（`run_graph`/`topological_layers`/`_run_one_node`/`_verify_and_maybe_regenerate`/`_ensure_node_row`/`_LockedConn`）、`src/work_graph_plan.py`（`_PROMPT_TEMPLATE`/`_normalize_graph_manifest`/`GRAPH_MANIFEST_SCHEMA`）、`src/e2e_pipeline.py`（`plan_documents`/`check_script_compiles`/`_check_prd_consistency`）、`src/settings_config.py`（`get_/upsert_concurrency_config` 先例）、`src/schema.py`（`migrate_expand_artifact_types` 先例）。 +**状态**: ✅ 已实现（2026-08-08）。实施顺序按第 11 节执行，`python3 -m unittest discover -s src`（692 tests）+ `discover -s supervisor -p "test_*.py"`（126 tests）全绿。实现过程中用真实测试发现并修正了本设计原文的几处问题（设计文档本身未回填，实现是最终准确来源）： +- §2 最初的 `on_fail.target_node_id` 字段设计，在实现前按用户反馈改成了从 `on_failure` 边读取（已反映在正文），但 §4.0（`topological_layers()` 必须过滤 `on_failure` 边）和 §8.3（`_validate_loop_back_targets` 改成校验边而不是字段）是**开始写代码之后**才发现遗漏并现场修的两处，不是设计阶段就想全的。 +- `_next_exec_node_id`（原设计称 `_count_prior_attempts`，按行数计数）在真实测试中暴露两个问题：① 按 DB 行数计数会把"resume 一个从未失败过的节点"误判成需要新开 attempt 后缀；② 只统计"自己是不是直接 loop_back 目标"会漏掉"作为下游节点被牵连重跑"的情况。最终实现改成按 `execution_event` 里真实的 loop_back 触发次数、且用 `_transitive_dependents` 判断是否被牵连来计数，两个问题都已修复并有专门测试覆盖。 +- `_run_one_node` 最初设计里"先统一调用 verify_fn 判断、再决定走哪条分支"的写法，会导致legacy（无 judgment 声明）节点的 verify_fn 被调用两次（`_verify_and_maybe_regenerate` 内部自己还会再调一次）——测试跑出来直接暴露，改成先判断 `has_own_judgment` 决定路径，只有新路径才提前调用 verify_fn。 +- `work_graph_plan.py::_PROMPT_TEMPLATE` 最初设计的"先 `.format()` 填 catalog_json、留 `{goal_text}` 给以后再填一次"写法有真实的转义 bug（catalog JSON 自带的花括号会被第二次 `.format()` 误当成占位符），实现时改成用字符串拼接嵌入 catalog、最终用 `.replace("{goal_text}", goal_text)` 而不是 `.format()` 填充目标文本，彻底绕开转义问题。 +- 顺带在真实测试中发现一个**不在本设计范围内、`work_node_run.py` 自己的既有缺口**：`run_work_node()` 每次调用都无条件往 `execution_log_file` 插入一行新记录，没有检查该 `node_exec_id` 是否已有记录——如果一个节点在 `execute_work_graph()` 真正返回之前就进程崩溃（不是"干净的 done"，也不是全新节点），下次对同一个 `node_exec_id` 调用 `run_work_node()` 会撞 `execution_log_file.node_exec_id` 的 UNIQUE 约束报错。这个缺口在 loop_back 机制之前就存在（任何节点崩溃在这个时间窗口都会触发,不是 loop_back 特有的),只是这次因为测试真实构造了"进程崩溃后重启"的场景才被发现——不在本次范围内修复,记录在案供后续单独立项。 + +--- + +## 1. 设计概览 + +现状（`work_graph_orchestrate.py`，见其自身 docstring 与代码）：`run_graph()` 把 `graph_manifest` 用 `topological_layers()` 分层，层间屏障、层内（G，2026-08-06）用 `ThreadPoolExecutor` 并发；每个节点的判断逻辑完全靠调用方传入的单一 `verify_fn(node, output) -> (ok, feedback)` 回调，失败处理固定死在 `_verify_and_maybe_regenerate()` 里——原地重新生成同一节点恰好一次，还失败就 `ConsistencyCheckFailedError`，没有"跳回更早节点"这条路径。 + +改造后新增两条能力，**都以 `graph_manifest` 节点自身携带的声明式字段驱动，不再要求调用方为每种 artifact_type 手写 Python 分支**： + +1. **`judgment`**：节点自己说"怎么判断我的输出对不对"（机械检查 or 触发一次额外 LLM 判断）。 +2. **`on_fail`**：节点自己说"判断失败了怎么办"（原地重试 / 跳回更早节点重新执行 / 直接判失败）。 + +**不改变的部分**（刻意保持稳定）： +- 每个节点仍然是"渲染成一个单节点 workflow 脚本 + 独立 `claude -p` 子进程调用"（`_make_run_fn`/`_render_node_script` 不变）。 +- G（并行执行）的层内并发机制、`_LockedConn`、`WorkGraphLayerError` 完全不变——回路是"层与层之间的控制流"，跟"层内节点并发执行"是正交的两层机制，互不冲突（详见第 4 节）。 +- 不声明 `judgment`/`on_fail` 的现有 `graph_manifest`（Coder 的合成单节点图）逐字节行为不变（FR-A8，第 8 节有完整核对表）。 +- `run_work_node()`/`work_node_run.py` 的断点续跑状态机完全不变，回路复用它现成的"新 node_id → 新行 → 真实重跑"机制（第 5 节），不需要改这一层。 + +## 2. `graph_manifest` 节点 schema 扩展 + +```jsonc +{ + "node_id": "benchmark", + "artifact_type": "benchmark", // 沿用现状；_GENERIC_PROMPT_TEMPLATE 路径用 task_prompt 代替，同样适用 + "depends_on": ["sdd"], + + // 新增，可选。缺省等价于 {"kind": "none"} —— 不做任何自动判断， + // 节点输出直接被接受（如果调用方仍然传了外部 verify_fn，外部的继续生效，见第 3 节）。 + "judgment": { + "kind": "mechanical", // "mechanical" | "llm" | "none" + "check_ref": "check_script_compiles" // 仅 kind="mechanical" 时必填，见第 3.1 节的白名单机制 + // kind="llm" 时改用： + // "judge_prompt": "Review the following content for ...: {content}" + // —— 由规划者（Planner 自己，或人工在固定链里手写）自己撰写， + // self-contained，不依赖调用方额外拼接上下文（同 _GENERIC_PROMPT_TEMPLATE + // 的 task_prompt 设计原则）。 + }, + + // 新增，可选。缺省等价于 {"action": "hard_fail"} 当没有 judgment 时； + // 当声明了 judgment 但没声明 on_fail 时，缺省等价于 {"action": "retry_self", "max_attempts": 1} + // ——即完全复刻现状 _verify_and_maybe_regenerate() 的行为（原地重试恰好一次）。 + "on_fail": { + "action": "loop_back", // "retry_self" | "loop_back" | "hard_fail" + "max_attempts": 3 // 可选，覆盖 FR-A5 配置表给出的默认值 + // 注意：没有 target_node_id 字段——action="loop_back" 的目标节点不是 + // node 自己声明的一个独立字段，而是从 graph_manifest 的 edges 里读出来 + // 的（见下）。retry_self/hard_fail 不需要任何 edge，target 就是自己/无。 + } +} +``` + +**2026-08-08 修订（用户反馈）**：失败路径的目标节点不应该是节点自己声明的、跟 `edges` 脱节的一个独立字段——"下一个节点是谁（成功路径）、失败了回到哪个节点（失败路径），都应该在规划这个节点的时候，用边（edge）定义好"，跟成功路径的 `depends_on`/`edges` 是同一套结构，不是节点身上另开的一个影子字段。所以 `on_fail.loop_back` 的目标改成**从一条新的 `edge_type: "on_failure"` 边读出来**，不再单独存 `target_node_id`： + +```jsonc +// graph_manifest 顶层 edges 数组： +{"from": "sdd", "to": "benchmark", "edge_type": "needs_judgment"}, // 成功路径：sdd 产出喂给 benchmark（既有语义，depends_on 的另一种表达） +{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"} // 失败路径：benchmark 判断失败时回到 sdd 重新执行 +``` + +即：一个节点的"下一步去哪"完全由它参与的 `edges` 决定——`needs_judgment`/`zero_token` 两种既有类型继续表达"成功之后数据往哪流"，新增的 `on_failure` 表达"判断失败之后控制流跳去哪"。节点自己的 `on_fail.action` 只声明"失败了要不要 loop_back"这个决策本身，不重复存目标是谁，避免两处数据不一致的风险（比如 `on_fail.target_node_id` 和实际 `edges` 没对上）。`GRAPH_MANIFEST_SCHEMA` 的 `edge_type` 枚举（`work_graph_plan.py:60`）加上 `"on_failure"`。 + +`GRAPH_MANIFEST_SCHEMA`（`work_graph_plan.py:33`）同步补上 `judgment`/`on_fail` 的形状 + `on_failure` edge_type（文档性质，跟现状一样不接入真正的 jsonschema 校验——`--json-schema` 和 `--agent` 不能同时用，见该文件已有说明）。 + +## 3. `run_graph()` 改造：judgment 解析 + +### 3.1 机械检查白名单——注入而不是硬编码 + +`work_graph_orchestrate.py` 现在对 `e2e_pipeline.py` **零依赖**（依赖方向是反过来的），不能直接 `import e2e_pipeline` 去拿 `check_script_compiles`（会成环）。机械检查改成调用方注入的白名单字典： + +```python +def run_graph( + conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, + build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, + verify_fn=None, sleep=time.sleep, now=time.time, max_workers=_DEFAULT_MAX_WORKERS, + mechanical_check_registry=None, # 新增：{check_ref: callable(content) -> (ok, detail)} +): +``` + +`e2e_pipeline.py` 的调用点传 `mechanical_check_registry={"check_script_compiles": check_script_compiles}`。`judgment.kind="mechanical"` 但 `check_ref` 不在注册表里，视为配置错误，直接在 `run_graph()` 一开始（还没跑任何节点前）校验并抛 `ValueError`——不要等跑到那个节点才发现拼错了名字，浪费一整条链的 token。**LLM 只能从白名单里选，不能发明新的机械检查**——这是有意的限制，不做"LLM 生成检查代码后拿去执行"这种沙箱执行的大工程。 + +### 3.2 LLM 判断——复用 `_check_prd_consistency` 的隔离机制，泛化提示词 + +```python +def _llm_judgment_verify_fn(judgment, org_node, target_folder_path, timeout=180): + """通用化的 _check_prd_consistency（e2e_pipeline.py）：同样的 self-target + worktree 隔离 + 独立 agent() 调用机制，但判断提示词来自 judgment['judge_prompt'] + （节点自己声明的，不是硬编码的 PRD 专属提示词），返回 schema 也通用化成 + {"ok": bool, "issues": [str, ...]}（PRD 检查原来的 "consistent"/"issues" 改名 + 成更通用的 "ok"，语义不变）。""" + is_self = paths.is_self_target(target_folder_path) + + def verify_fn(node, output): + node_exec_id = "judgment-" + 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: + prompt = judgment["judge_prompt"].format(content=output["content"]) + script_name = "graphlemon-judgment-" + uuid.uuid4().hex + script_content = ( + "export const meta = {{ name: {name}, description: 'graphlemon node judgment' }}\n\n" + "const result = await agent({prompt}, {{ agentType: {org_node_json}, schema: {schema} }})\n\n" + "return result\n" + ).format( + name=json.dumps(script_name), prompt=json.dumps(prompt), + org_node_json=json.dumps(org_node), schema=json.dumps(_JUDGMENT_SCHEMA), + ) + work_graph_execute.write_workflow_script(cwd, script_name, script_content) + result = work_graph_execute.execute_work_graph(cwd, script_name, {}, _JUDGMENT_SCHEMA, timeout=timeout) + if worktree_path: + work_node_run.ensure_worktree_committed(worktree_path, node_exec_id) + work_node_run.ensure_worktree_committed(target_folder_path, node_exec_id + "-main-checkout-drift") + work_node_run.merge_worktree(target_folder_path, node_exec_id) + ok = _coerce_bool(result["ok"]) + return ok, (result.get("issues") or []) + finally: + if worktree_path: + work_node_run.remove_worktree(target_folder_path, worktree_path) + + return verify_fn + + +_JUDGMENT_SCHEMA = { + "type": "object", "required": ["ok", "issues"], + "properties": {"ok": {"type": "boolean"}, "issues": {"type": "array", "items": {"type": "string"}}}, +} +``` + +`_coerce_bool`（原属 `e2e_pipeline.py`，见其 2026-08-02 的"LLM 把 boolean 回成字符串"教训）需要下沉成 `work_graph_orchestrate.py` 自己的小工具函数（或 `paths.py` 这类共享工具模块），两边都用同一份，不重复实现。`e2e_pipeline.py::_check_prd_consistency`/`_prd_consistency_verify_fn` 保留不动（FR-A8 向后兼容，现有固定链路径不强制迁移），新代码只是提供一个可选的、更通用的平行实现。 + +### 3.3 节点级 verify_fn 解析 + +```python +def _resolve_node_verify_fn(node, external_verify_fn, org_node, target_folder_path, mechanical_check_registry): + judgment = node.get("judgment") + if not judgment or judgment.get("kind", "none") == "none": + return external_verify_fn # 完全现状：没声明 judgment，谁传了外部 verify_fn 就用谁的（可能是 None） + if judgment["kind"] == "mechanical": + check_fn = (mechanical_check_registry or {}).get(judgment["check_ref"]) + if check_fn is None: + raise ValueError("unknown mechanical check_ref: {!r}".format(judgment["check_ref"])) + def verify_fn(node, output, _check_fn=check_fn): + ok, detail = _check_fn(output["content"]) + return ok, ([detail] if detail else None) + return verify_fn + if judgment["kind"] == "llm": + return _llm_judgment_verify_fn(judgment, org_node, target_folder_path) + raise ValueError("unknown judgment.kind: {!r}".format(judgment["kind"])) +``` + +`_run_one_node()` 改成对每个节点各自调用 `_resolve_node_verify_fn(...)` 拿到"这个节点该用哪个 verify_fn"，而不是像现在这样整张图共用调用方传入的同一个 `verify_fn`。**向后兼容关键点**：现有调用方（`plan_documents`/`validate_implementation_with_own_graph`）继续传它们现在的 `_combined_planner_verify_fn(...)`/`_test_script_verify_fn(...)` 作为 `external_verify_fn`，只要节点没有 `judgment` 字段（现状 100% 如此），`_resolve_node_verify_fn` 原样返回这个外部函数，行为逐字节不变（FR-A8 验收标准 5）。 + +## 4. `on_fail` 执行：回路引擎 + +### 4.0 前提修复：`topological_layers()` 必须过滤 `on_failure` 边（FR-A7） + +**这是把 loop_back 目标从节点字段改成 `edges` 里的一条边（第 2 节 2026-08-08 修订）之后，必须同步修的一处，否则会引入真环**：现有 `topological_layers()`（`work_graph_orchestrate.py:196-200`）把 `graph_manifest["edges"]` 里**每一条边、不分 `edge_type`**都当成数据依赖合并进 `deps`： + +```python +for edge in graph_manifest.get("edges", []): + to_id, from_id = edge.get("to"), edge.get("from") + if to_id in deps and from_id in nodes: + deps[to_id].add(from_id) +``` + +如果不改，一条 `{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}` 会被这段代码当成"`sdd` 依赖 `benchmark`"塞进 `deps["sdd"]`——而 `benchmark` 本来就依赖 `sdd`（正常数据依赖），两条方向相反的依赖凑在一起就是一个真环，`topological_layers()` 会在图规划阶段还没触发任何回路之前，就先被静态校验直接拒绝，FR-A7 的验收标准 4 会失败。 + +修复：合并 `deps` 时跳过 `edge_type == "on_failure"` 的边——**只有 `on_failure` 需要特殊处理**，`zero_token`/`needs_judgment` 两种既有类型继续按现状合并（它们表达的就是真实数据依赖）： + +```python +def topological_layers(graph_manifest): + nodes = {n["node_id"]: n for n in graph_manifest["nodes"]} + deps = {node_id: set(n.get("depends_on") or []) for node_id, n in nodes.items()} + for edge in graph_manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue # 新增：控制流边不参与静态拓扑排序 + to_id, from_id = edge.get("to"), edge.get("from") + if to_id in deps and from_id in nodes: + deps[to_id].add(from_id) + ... # 其余逻辑不变 +``` + +`topological_order()`（薄封装）不用改。这条修复是本设计里**唯一一处直接改动 G（并行执行）已经上线的现有函数**的地方，改动是纯粹的"多一个 `continue` 条件"，不影响任何现有（不含 `on_failure` 边的）`graph_manifest` 的分层结果——`src/test_work_graph_orchestrate.py` 里 G 自己的既有测试用例不需要改一行断言即可继续通过。 + +### 4.1 关键简化：`retry_self` 是 `loop_back` 的特例 + +`retry_self` ≡ `loop_back` 且 `target_node_id` = 自己。原因：`retry_self` 触发时这个节点本身还没成功过，不可能已经有下游节点跑完并读取过它的（不存在的）输出，所以"重跑 target 到失败节点之间的下游链"这条规则应用到"target=自己"时，受影响集合就是 `{自己}`，跟现状的"原地重跑"完全等价。**一套机制两种声明方式**，不用维护两条独立代码路径。 + +### 4.2 受影响子图：只重跑真正的下游依赖，不是整层 + +需求文档非目标已声明"不追求完全通用的图执行框架"，但"回路触发时把同层不相关的兄弟节点也无谓重跑一遍"没必要，也和已完成节点白白重花 token 相悖，成本可控且实现不复杂： + +```python +def _transitive_dependents(graph_manifest, node_id): + """node_id 自身 + 所有直接/间接依赖它的下游节点 id 集合（沿 depends_on/edges + 正向遍历）。用于 loop_back 时判断哪些节点的输出已经"过期"、必须重跑。""" + dependents = {} # node_id -> [直接依赖它的节点 id, ...] + for node in graph_manifest["nodes"]: + dependents.setdefault(node["node_id"], []) + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} + for edge in graph_manifest.get("edges", []): + to_id, from_id = edge.get("to"), edge.get("from") + if to_id in deps and from_id in deps: + deps[to_id].add(from_id) + for node_id_, dep_set in deps.items(): + for dep in dep_set: + dependents.setdefault(dep, []).append(node_id_) + + affected, stack = set(), [node_id] + while stack: + current = stack.pop() + if current in affected: + continue + affected.add(current) + stack.extend(dependents.get(current, [])) + return affected + + +def _on_failure_target(graph_manifest, node_id): + """节点 node_id 声明了 on_fail.action='loop_back' 时，目标节点不是节点 + 自己存的字段，而是 graph_manifest['edges'] 里唯一一条 + {"from": node_id, "to": ..., "edge_type": "on_failure"} 边的 "to"—— + 2026-08-08 修订：失败路径跟成功路径一样，都是"边"，不是节点身上另开 + 的一个跟 edges 脱节的影子字段。恰好 0 条或 2+ 条这样的边是规划错误， + 在 _validate_loop_back_targets（8.3 节）里提前拦截，这里假设已经过 + 校验，直接找第一条匹配的。""" + for edge in graph_manifest.get("edges", []): + if edge.get("from") == node_id and edge.get("edge_type") == "on_failure": + return edge["to"] + raise ValueError("node {} declares on_fail.action='loop_back' but has no on_failure edge".format(node_id)) +``` + +### 4.3 层级回退循环——`run_graph()` 主循环改造 + +```python +class _LoopBackRequested(Exception): + """节点内部信号：这个节点的 on_fail.action=loop_back 触发了，需要外层 + run_graph() 主循环回退重跑，不是真正的失败——不通过正常返回值传递是 + 因为它可能发生在 ThreadPoolExecutor 的 worker 线程里（层内并发场景）， + 异常是唯一能干净地从 future.result() 里带出这个信号的方式。""" + def __init__(self, source_node_id, target_node_id, feedback): + self.source_node_id = source_node_id + self.target_node_id = target_node_id + self.feedback = feedback + super().__init__("node {} requests loop_back to {}".format(source_node_id, target_node_id)) + + +def run_graph(..., mechanical_check_registry=None): + ensure_placeholder_skill(conn, skill_id, org_node) + effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path) + layers = topological_layers(graph_manifest) + node_layer_index = {node["node_id"]: i for i, layer in enumerate(layers) for node in layer} + outputs = {} + locked_conn = _LockedConn(conn) + attempt_counts = {} # (target_node_id, source_node_id) -> 已尝试次数，本次 run_graph() 调用内的内存计数 + # 断点续跑时从 execution_event 重建，见第 6.3 节 + + layer_index = 0 + while layer_index < len(layers): + layer = layers[layer_index] + try: + _run_layer( + conn, locked_conn, run_id, org_node, skill_id, target_folder_path, layer, outputs, + build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, + verify_fn, mechanical_check_registry, max_workers, sleep, now, + ) + except _LoopBackRequested as sig: + key = (sig.target_node_id, sig.source_node_id) + attempt_counts[key] = attempt_counts.get(key, 0) + _loop_attempt_count_from_events(conn, run_id, key) + limit = _resolve_loop_limit(conn, org_node, graph_manifest, sig.target_node_id) + if attempt_counts[key] > limit: + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) + conn.commit() + raise ConsistencyCheckFailedError( + "loop_back from {} to {} exceeded max_attempts={}".format( + sig.source_node_id, sig.target_node_id, limit + ), + node_id=sig.source_node_id, + ) + _record_loop_back_event(conn, run_id, sig, attempt_counts[key]) + affected = _transitive_dependents(graph_manifest, sig.target_node_id) + for affected_id in affected: + outputs.pop(affected_id, None) # 强制这些节点在重跑时被当成"未完成" + layer_index = node_layer_index[sig.target_node_id] + continue + layer_index += 1 + + return outputs +``` + +`_run_layer(...)` 是现有 `run_graph()` 主循环体（单节点同步 / 多节点 `ThreadPoolExecutor`）原样搬过来的辅助函数，唯一的改动是：**只重跑本轮 `affected` 集合里的节点**（第一次进入某层时 `affected` 概念不适用，正常跑全部；loop_back 回退重入某层时，只有 `outputs` 里被 `.pop()` 掉的节点需要重新执行，其余节点直接复用已有的 `outputs[node_id]`，不重新调用 `_run_one_node`）——`outputs.pop()` 天然标记了"这个需要重跑"，`_run_layer` 判断 `node["node_id"] not in outputs` 决定要不要真的执行这个节点，跳过的节点保留原值。 + +`_run_one_node()` 里判断 `verify_fn` 失败后该走哪条路径（原地重试 vs 抛 `_LoopBackRequested`）： + +```python +def _run_one_node(..., node, outputs, ...): + node_id = node["node_id"] + # 每次真正执行都用一个新的 attempt 后缀 node_id，绝不复用同一个已经 + # 'done' 的 node_id——_ensure_node_row 按 (run_id, node_id) 去重，复用 + # 同一个 node_id 会被 run_work_node 的断点续跑短路成"已完成，直接返回 + # 旧输出"，这正是必须避免的（详见 5 节）。 + attempt = _count_prior_attempts(conn, run_id, node_id) + 1 + exec_node_id = node_id if attempt == 1 else "{}-attempt{}".format(node_id, attempt) + + node_exec_id = _ensure_node_row(conn, run_id, exec_node_id, org_node, skill_id, ..., {"node_id": node_id}) + prompt = build_prompt(node, outputs) + _loop_history_prompt_suffix(conn, run_id, node_id) + run_fn = _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) + output = work_node_run.run_work_node(conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, ...) + + resolved_verify_fn = _resolve_node_verify_fn(node, verify_fn, org_node, target_folder_path, mechanical_check_registry) + if resolved_verify_fn is None: + return output + ok, feedback = resolved_verify_fn(node, output) + if ok: + return output + + on_fail = node.get("on_fail") or {"action": "retry_self" if node.get("judgment") else "hard_fail"} + action = on_fail.get("action", "retry_self") + if action == "hard_fail" or (action == "retry_self" and not node.get("judgment")): + # 完全现状路径：没声明 judgment 的老节点，外部 verify_fn 失败时走 + # 原有的 _verify_and_maybe_regenerate()（恰好一次原地重试），逐字节不变。 + return _verify_and_maybe_regenerate(conn, run_id, org_node, skill_id, target_folder_path, + node, node_id, prompt, output, input_schema, output_schema, + timeout, needs_worktree, resolved_verify_fn, sleep, now) + target = node_id if action == "retry_self" else _on_failure_target(graph_manifest, node_id) + raise _LoopBackRequested(source_node_id=node_id, target_node_id=target, feedback=feedback) +``` + +（`_run_one_node` 需要拿到 `graph_manifest` 才能查 `on_failure` 边——现有签名没有这个参数，改造时顺带传进去；`retry_self` 完全不查边，`target_node_id` 直接等于自己，第 4.1 节已经说明原因。） + +**关键取舍**：`hard_fail`、以及"没声明 `judgment` 的老节点走 `retry_self`"这两种情况，继续复用现有的 `_verify_and_maybe_regenerate()` 一字不改——不是所有失败路径都要改造成新机制，只有节点**主动声明了 `judgment`**（即 Part B 自规划出的新节点，或 Part A 上线后手工升级的固定链节点）才会走新的 `_LoopBackRequested` 回路路径。这保证 FR-A8 的向后兼容不是"理论上兼容"而是"代码路径都没变过"。 + +## 5. 断点续跑兼容性：新 attempt 后缀就是全部答案 + +不需要专门为回路设计新的续跑逻辑。原理：`_run_one_node()` 每次真正执行前，都先查询"这个 node_id 已经有几次真正的执行尝试"（`_count_prior_attempts`，扫 `work_node_execution` 里 `node_id` 匹配 `original_id` 或 `original_id-attemptN` 模式的行数），用下一个序号构造 `exec_node_id`。进程崩溃重启后重新调用 `run_graph()`： + +- 还没跑到的层：`outputs` 里没有对应 key，`_run_layer` 判断需要执行，正常走一遍——`_count_prior_attempts` 会看到之前留下的部分尝试记录（如果崩溃发生在某次 attempt 执行中途），用下一个序号继续，不会跟已存在的行冲突（`_ensure_node_row` 按 `exec_node_id` 去重，`exec_node_id` 本身按 attempt 序号递增，天然不冲突）。 +- 已经 `done` 的 attempt：`run_work_node()` 现有的"已完成短路"逻辑对每个 `exec_node_id` 各自生效，不需要改。 +- `attempt_counts`（内存字典，第 4.3 节）：崩溃后是空的，但 `run_graph()` 一旦真的触发 `_LoopBackRequested` 时会先调用 `_loop_attempt_count_from_events(conn, run_id, key)`（第 6.3 节，从 `execution_event` 里查询真实发生过的次数），不依赖内存状态——重启后第一次遇到某个 loop_back key 时，从 DB 查出来的次数就是崩溃前的真实累计值，不会被内存清零重置为 0。 + +## 6. 循环上限配置（FR-A5） + +### 6.1 新表（`settings_config.py` 风格） + +```sql +-- schema.py 新增 +CREATE TABLE work_graph_loop_limit_config ( + config_id TEXT PRIMARY KEY, -- "{org_node}:{node_type}" / "{org_node}:*" / "default" + max_loop_attempts INTEGER NOT NULL, + updated_by TEXT, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP +); +``` + +```python +# settings_config.py 新增，跟 get_/upsert_concurrency_config 完全同构 +def get_loop_limit_config(conn, config_id): + row = conn.execute( + "SELECT config_id, max_loop_attempts, updated_at FROM work_graph_loop_limit_config WHERE config_id = ?", + (config_id,), + ).fetchone() + return None if row is None else {"config_id": row[0], "max_loop_attempts": row[1], "updated_at": row[2]} + + +def upsert_loop_limit_config(conn, config_id, max_loop_attempts, updated_by): + if max_loop_attempts < 1: + raise InvalidParamError("max_loop_attempts must be >= 1, got {!r}".format(max_loop_attempts)) + exists = conn.execute( + "SELECT 1 FROM work_graph_loop_limit_config WHERE config_id = ?", (config_id,) + ).fetchone() is not None + if exists: + conn.execute( + "UPDATE work_graph_loop_limit_config SET max_loop_attempts = ?, updated_by = ?, " + "updated_at = CURRENT_TIMESTAMP WHERE config_id = ?", + (max_loop_attempts, updated_by, config_id), + ) + else: + conn.execute( + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) VALUES (?, ?, ?)", + (config_id, max_loop_attempts, updated_by), + ) + conn.commit() + return get_loop_limit_config(conn, config_id) +``` + +### 6.2 查找顺序 + 硬编码兜底 + +```python +# work_graph_orchestrate.py +_DEFAULT_LOOP_LIMIT = 3 # 只在配置表连 "default" 行都没有时才会用到（比如全新数据库还没跑过 seed） + +def _resolve_loop_limit(conn, org_node, graph_manifest, target_node_id): + node = next(n for n in graph_manifest["nodes"] if n["node_id"] == target_node_id) + node_level = (node.get("on_fail") or {}).get("max_attempts") + if node_level is not None: + return node_level # 节点自己声明的优先级最高（FR-A5） + node_type = node.get("artifact_type", target_node_id) + for config_id in ("{}:{}".format(org_node, node_type), "{}:*".format(org_node), "default"): + config = settings_config.get_loop_limit_config(conn, config_id) + if config: + return config["max_loop_attempts"] + return _DEFAULT_LOOP_LIMIT +``` + +初始 seed：`schema.create_schema()`/迁移里插入一条 `config_id="default", max_loop_attempts=3` 的行（跟 `org_loop_route._LOOP_LIMIT=3` 取值一致，作为一个"暂时先抄现有跨 Org loop 的经验值，后续按真实调用数据调整"的起点——呼应需求文档「7. 风险」第二条，这不是拍脑袋，是有意选择一个已经在生产验证过的数量级作为起点，且明确设计成之后可以不改代码调整）。 + +## 7. loop 历史持久化（FR-A4）：`execution_event` 迁移 + +### 7.1 新增 `event_type`：`loop_back_triggered` + +`execution_event.event_type` 是 CHECK 约束枚举（`schema.py:262-266`），SQLite 无法直接 `ALTER`，沿用 `migrate_expand_artifact_types` 的 rebuild-copy-swap 手法： + +```python +# schema.py 新增 +def migrate_add_loop_back_event_type(conn): + """2026-08-08: 给 execution_event.event_type 的 CHECK 枚举加 + 'loop_back_triggered'，用于记录 Work Graph 内部回路——见 + docs/todo/work-graph-self-planning-and-loop-back-design.md §7。 + 幂等，安全对已有数据库无条件调用（同 migrate_expand_artifact_types 的写法）。 + """ + row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='execution_event'").fetchone() + if row is None or "'loop_back_triggered'" in row[0]: + return + conn.executescript( + """ + CREATE TABLE execution_event_new ( + event_id TEXT PRIMARY KEY, + project_id TEXT NOT NULL REFERENCES project(project_id), + task_id TEXT REFERENCES task(task_id), + run_id TEXT REFERENCES work_graph_run(run_id), + node_exec_id TEXT REFERENCES work_node_execution(node_exec_id), + event_type TEXT NOT NULL CHECK (event_type IN ( + 'task_status_changed', 'node_status_changed', 'artifact_created', + 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', + 'log_appended', 'quota_wait_started', 'loop_back_triggered' + )), + payload TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + INSERT INTO execution_event_new SELECT * FROM execution_event; + DROP TABLE execution_event; + ALTER TABLE execution_event_new RENAME TO execution_event; + CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); + CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); + """ + ) + conn.commit() +``` + +`_ensure_schema()`（`backend_server.py:1020`）加一行无条件调用，跟 `migrate_expand_artifact_types`/`migrate_relax_org_node_constraints` 并列——这正是 2026-08-06 那次"忘记接进 `_ensure_schema`导致生产 500"的教训要求的（该函数自己的注释已经把这条约定写死）。 + +### 7.2 写入 + 查询 + +```python +def _record_loop_back_event(conn, run_id, sig, attempt_number): + task_id, project_id = conn.execute( + "SELECT task_id, project_id FROM work_graph_run wgr JOIN task t ON wgr.task_id = t.task_id " + "WHERE wgr.run_id = ?", (run_id,) + ).fetchone() + conn.execute( + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " + "VALUES (?, ?, ?, ?, 'loop_back_triggered', ?)", + (str(uuid.uuid4()), project_id, task_id, run_id, json.dumps({ + "source_node_id": sig.source_node_id, "target_node_id": sig.target_node_id, + "attempt_number": attempt_number, "feedback": sig.feedback, + })), + ) + conn.commit() + + +def _loop_attempt_count_from_events(conn, run_id, key): + target_node_id, source_node_id = key + rows = conn.execute( + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered'", (run_id,) + ).fetchall() + return sum( + 1 for (payload,) in rows + if json.loads(payload)["target_node_id"] == target_node_id + and json.loads(payload)["source_node_id"] == source_node_id + ) + + +def _loop_history_prompt_suffix(conn, run_id, node_id): + """target_node_id 重新执行时，把它过去每一轮被打回的完整历史（不只 + 最近一轮）拼进提示词——FR-A4。""" + rows = conn.execute( + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered' " + "ORDER BY created_at", (run_id,) + ).fetchall() + history = [json.loads(p) for (p,) in rows if json.loads(p)["target_node_id"] == node_id] + if not history: + return "" + lines = [ + "Attempt {}: rejected because: {}".format(h["attempt_number"], "; ".join(h["feedback"] or [])) + for h in history + ] + return "\n\nThis node was sent back for revision before, with this history:\n" + "\n".join(lines) +``` + +不新增表，`execution_event` 一张表回答"这个节点被打回几次、每次谁触发的、为什么、按什么顺序"——满足决策 #4，且天然带 `run_id`/`node_exec_id`/`created_at`，现有图可视化机制（`v_graph_visualizer`，若按 `run_id` 拉取 `execution_event` 渲染）不需要新的数据源就能把 `source_node_id → target_node_id` 画成一条回路边。 + +## 8. Part B：Planner 自规划节点集合 + +### 8.1 节点类型清单——数据，不是硬编码提示词字符串（FR-B2） + +```python +# work_graph_plan.py 新增 +_PLANNER_NODE_CATALOG = [ + {"artifact_type": "brd", "description": "商业需求文档...", "judgment": {"kind": "none"}}, + {"artifact_type": "prd", "description": "产品需求文档...", + "judgment": {"kind": "llm", "judge_prompt": _PRD_CONSISTENCY_JUDGE_PROMPT}}, + {"artifact_type": "sdd", "description": "详细设计文档...", "judgment": {"kind": "none"}}, + {"artifact_type": "benchmark", "description": "验收基准...", "judgment": {"kind": "none"}}, + {"artifact_type": "test_dataset", "description": "测试数据集（可选，goal 不需要时回复 N/A）...", + "judgment": {"kind": "none"}}, + {"artifact_type": "test_script", "description": "可执行验收脚本（可选，goal 不需要时回复 N/A）...", + "judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}}, +] +``` + +`_PRD_CONSISTENCY_JUDGE_PROMPT` 是现有 `_PRD_CONSISTENCY_PROMPT`（`e2e_pipeline.py:72`）改写成 `judge_prompt` 的通用形状（`{content}` 占位符替代原来的 `{prd_content}`），供 Planner 在自规划时"抄"进它给 prd 节点声明的 `judgment` 里——这份清单只是给 LLM 的**参考建议**，不是强制：Planner 完全可以给某个节点声明跟清单建议不同的 `judgment`/`on_fail`（FR-B4 已确认"全部自由规划"），清单只是降低它每次都要从零发明判断方式的成本。 + +### 8.2 新 `_PROMPT_TEMPLATE` + +```python +_PROMPT_TEMPLATE = ( + "Plan (do not execute) the Work Graph for this goal: {goal_text}\n" + "Available node types (use any subset, in any order that makes sense -- " + "this goal may not need all of them):\n{catalog_json}\n" + "For each node you include, decide: node_id, artifact_type, depends_on, " + "a judgment config (how to tell if this node's output is acceptable -- you " + "may reuse a catalog entry's suggested judgment, or write your own 'llm' " + "judge_prompt), and an on_fail config (what happens if judgment fails: " + "retry_self, loop_back, or hard_fail). If a node's on_fail.action is " + "'loop_back', you MUST also add a corresponding edge in the top-level " + "'edges' array with edge_type 'on_failure', from that node to the exact " + "earlier node_id it should return to -- the target must be a real " + "upstream dependency of this node (something that genuinely produced " + "input this node used), not an arbitrary node. This mirrors how the " + "success path already works: every node's 'yes, this succeeded, what's " + "next' is expressed as a normal dependency edge, and 'no, this failed, " + "go back to X' is expressed the same way, just with edge_type " + "'on_failure' instead. Every other edge needs edge_type: 'zero_token' " + "for pure aggregation/reduce steps, 'needs_judgment' for anything " + "requiring the agent to actually reason about content.\n" + "End your reply with the graph_manifest as a single fenced ```json code " + "block shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no " + "extra wrapper key." +).format(catalog_json=json.dumps(_PLANNER_NODE_CATALOG), goal_text="{goal_text}") +``` + +（`.format()` 两次是因为 `catalog_json` 在模块加载时就能确定、`goal_text` 要留到调用时才填——用 `{{`/`}}` 转义加上先格式化一次 `catalog_json`，再对外暴露带 `{goal_text}` 占位符的最终模板字符串，写法上等价于现有 `_GENERIC_PROMPT_TEMPLATE` 的处理方式，不是新模式。） + +### 8.3 `_normalize_graph_manifest` 的变化：删除，不是加固 + +`_CANONICAL_DOC_CHAIN` 常量、以及"当六个/五个规范类型全部出现时强制线性依赖顺序"那段逻辑（`work_graph_plan.py`，2026-08-07 已经因为禁用 test_script 缩成 5 元组）**整体删除**——FR-B4 已确认"全部自由规划，不保留任何代码级固定顺序兜底"。`_normalize_graph_manifest` 保留的部分： +- `id`/`node_id` 键名兼容（2026-08-01 真实教训）。 +- 从 `edges` 反推 `depends_on`（同上）。 + +新增的部分： +- **2026-08-08 修订（用户反馈）**：失败路径的目标节点不是节点身上一个跟 `edges` 脱节的独立字段，而是跟成功路径一样，由规划这个节点的时候一并定好的边决定——"yes 走哪个节点、no 走哪个节点，都是边定义好的"。所以校验对象变成：每个声明了 `on_fail.action="loop_back"` 的节点，必须**恰好有一条**从它出发、`edge_type="on_failure"` 的边（见第 4.2 节 `_on_failure_target`），且这条边指向的节点必须是该节点的（直接或间接）上游依赖——LLM 规划时可能"跳"到一个跟自己毫无依赖关系的节点（逻辑上没有意义：loop_back 的语义是"回到更早、真正影响了我的节点"），这种情况在 `_normalize_graph_manifest` 里直接拒绝并抛 `PlanningFailedError`（复用现有异常类），比让它在 `run_graph()` 执行到一半才报错更早暴露问题。 + +```python +def _validate_loop_back_targets(graph_manifest): + node_ids = {n["node_id"] for n in graph_manifest["nodes"]} + for node in graph_manifest["nodes"]: + on_fail = node.get("on_fail") or {} + if on_fail.get("action") != "loop_back": + continue + node_id = node["node_id"] + on_failure_edges = [ + e for e in graph_manifest.get("edges", []) + if e.get("from") == node_id and e.get("edge_type") == "on_failure" + ] + if len(on_failure_edges) != 1: + raise PlanningFailedError( + "node {} declares on_fail.action='loop_back' but has {} on_failure edge(s), expected exactly 1".format( + node_id, len(on_failure_edges) + ) + ) + target = on_failure_edges[0]["to"] + if target not in node_ids: + raise PlanningFailedError( + "node {}'s on_failure edge targets {!r}, which does not exist in this graph_manifest".format( + node_id, target + ) + ) + if target not in _transitive_ancestors(graph_manifest, node_id): + raise PlanningFailedError( + "node {}'s on_failure edge targets {!r}, which is not an upstream dependency -- " + "loop_back must target a node that genuinely influenced this one".format(node_id, target) + ) +``` + +（`_transitive_ancestors` 是 `_transitive_dependents`——第 4.2 节，`work_graph_orchestrate.py`——的反向版本；两边各自独立实现一份即可，`work_graph_plan.py` 不需要因此新引入对 `work_graph_orchestrate.py` 的依赖，两个模块现在也没有互相 import，保持这个边界。`_on_failure_target`——第 4.2 节——运行时可以放心假设"恰好一条"已经在这里校验过，不用再处理 0 条/多条的情况。） + +### 8.4 `plan_documents()` 调用点改动 + +`e2e_pipeline.py::plan_documents()` 不再需要 `_combined_planner_verify_fn`（PRD 一致性检查 + test_script 编译检查两个手写分支）——改成 `verify_fn=None`，因为自规划出的每个节点都自带 `judgment`，`run_graph()` 内部会自动解析。`mechanical_check_registry={"check_script_compiles": check_script_compiles}` 照常传入。旧的 `_combined_planner_verify_fn`/`_prd_consistency_verify_fn`/`_test_script_verify_fn` 函数**不删除**——`validate_implementation_with_own_graph()`（Validator 自己的硬编码 3/2 节点图，本轮不改，见需求文档非目标）继续用它们，且它们仍然是"外部 verify_fn 向后兼容路径"的真实调用方，删除会破坏 FR-A8。 + +## 9. 向后兼容性核对表（FR-A8） + +| 现有调用点 | 是否声明 `judgment`/`on_fail` | 改造后走的路径 | +|---|---|---| +| `implement_code()` 的合成单节点图 | 否 | `_resolve_node_verify_fn` 直接返回 `None`（该调用点本来就不传 `verify_fn`），零改动 | +| `validate_implementation_with_own_graph()`（`_VALIDATOR_MANIFEST`） | 否 | `_resolve_node_verify_fn` 返回外部 `_test_script_verify_fn(...)`，失败时走 `hard_fail`/现状 `_verify_and_maybe_regenerate`，逐字节不变 | +| `work_graph_fork.py`（`_GENERIC_PROMPT_TEMPLATE` 路径） | 否 | 同上，无 `verify_fn` 传入，无判断 | +| `plan_documents()`（Part B 改造后） | 是（每个节点自带） | 走新的 `judgment`/`on_fail` 解析路径 | + +## 10. 测试计划（对应需求文档验收标准 1-10） + +| 验收标准 | 测试方式 | +|---|---| +| 1-2. loop_back 累积历史 + 触发上限 | mock `execute_work_graph`，构造 A→B、B 声明 `on_fail=loop_back(target=A)`，B 连续失败 N 次；断言 A 被重跑 N 次、`execution_event` 里有 N 条 `loop_back_triggered`、A 第 N 次收到的 prompt 包含前 N-1 次的 feedback 文本 | +| 3. 崩溃重启后正确恢复轮次 | 跑到第 2 次 loop_back 时中断（mock 抛异常模拟进程退出），重新调用 `run_graph()`，断言 `_loop_attempt_count_from_events` 从 DB 读到 2，不是从 0 开始 | +| 4. 静态图校验不因 loop_back 报环 | 一个声明了 `on_fail.action="loop_back"` 且带一条指向上游节点的 `on_failure` 边的 `graph_manifest`，`topological_layers()` 正常返回分层，不抛 `ValueError`（因为 `on_failure` 边类型根本不参与 `topological_layers` 的依赖计算，只有 `depends_on`/普通数据依赖 edge 参与） | +| 5. 未声明字段的图逐字节不变 | 现有 `test_work_graph_orchestrate.py`/`test_e2e_pipeline.py` 全部既有用例不改断言即通过 | +| 6-7. Part B 自由规划 + 健壮性兜底 | 同需求文档，mock LLM 规划响应缺 test_dataset / 缺 depends_on，断言分别正确处理 | +| 8. test_script 可选 | mock 规划响应包含 test_script 节点，断言正常规划执行 | +| 9. 按 org_node/node_type 配置上限生效 | `settings_config.upsert_loop_limit_config` 写两条不同配置，构造两个 loop_back 场景，断言各自在配置的上限触发 | +| 10. loop 历史只在 `execution_event` | 触发若干轮 loop_back 后，只查 `execution_event`（不查任何新表）复原完整历史 | +| 新增：`_validate_loop_back_targets` 拒绝非法目标 | 分别构造"`on_failure` 边指向的节点不是上游依赖"、"声明 `loop_back` 但没有 `on_failure` 边"、"同一个节点有 2 条 `on_failure` 边"三种场景，断言 `plan_work_graph()` 都抛 `PlanningFailedError` | +| 新增：mechanical_check_registry 未注册报错 | `judgment.check_ref` 指向一个不存在的名字，断言 `run_graph()` 在执行任何节点前就报错，不是跑到那个节点才发现 | +| 新增：`topological_layers()` 正确忽略 `on_failure` 边（4.0 节修复） | 构造 A→B（普通依赖）+ B→A（`on_failure` 边）的 `graph_manifest`，断言 `topological_layers()` 返回 `[[A], [B]]`（不因为 B→A 那条边被误判成环）；另跑一遍 G 既有的 `test_work_graph_orchestrate.py` 全部用例确认零回归 | + +## 11. 实施步骤建议 + +1. `schema.py` 两处迁移（`work_graph_loop_limit_config` 建表 + `migrate_add_loop_back_event_type`）+ `settings_config.py` 两个新函数 + `_ensure_schema()` 接入——纯数据层改动，先落地，不碰任何执行逻辑。 +2. `work_graph_orchestrate.py`：`_resolve_node_verify_fn`/机械检查注册表注入/`_llm_judgment_verify_fn`——先只做"judgment 解析"，不做 loop_back（这一步验证：没有 `on_fail` 时行为完全不变，第 9 节核对表跑一遍）。 +3. `work_graph_orchestrate.py`：`_transitive_dependents`/`_LoopBackRequested`/`run_graph()` 主循环回退逻辑/attempt 后缀机制——loop_back 的核心，单独用 mock 场景验证第 10 节测试计划 1-5、9、10。 +4. `work_graph_plan.py`：删除 `_CANONICAL_DOC_CHAIN` 强制顺序逻辑、`_PLANNER_NODE_CATALOG`、新 `_PROMPT_TEMPLATE`、`_validate_loop_back_targets`——Part B，第 10 节测试计划 6-8、新增两项。 +5. `e2e_pipeline.py::plan_documents()` 切到 `verify_fn=None` + `mechanical_check_registry`，端到端跑一次 mock 场景确认整条链路。 +6. 真实场景验证（`RUN_REAL_AGENT_E2E=1`，小心控制次数/成本）：观察一次真实 Planner 自规划调用产出的 `graph_manifest` 长什么样，是否合理、是否触发过 `PlanningFailedError`。 diff --git a/docs/todo/work-graph-self-planning-and-loop-back-requirements.md b/docs/todo/work-graph-self-planning-and-loop-back-requirements.md new file mode 100644 index 0000000..9d7375d --- /dev/null +++ b/docs/todo/work-graph-self-planning-and-loop-back-requirements.md @@ -0,0 +1,106 @@ +# Planner 动态自规划 Work Graph（含节点判断与失败回路）——需求文档 + +**日期**: 2026-08-08 +**性质**: 需求文档（BRD/PRD 层面），不含详细技术设计。回应用户 2026-08-07/08-08 两轮讨论：先是"要不要让 Planner 自己判断要不要有 test_script 节点"，深入后明确为一个更完整的诉求——Planner 第一步应该是自己规划 Work Graph（要不要 brd/prd/sdd/benchmark/test 的组合、未来还有更多 tool/skill 可组合调用），并为每个节点规划输入输出边约束、判断成功/失败的方式，失败时把结果送回更早的节点形成回路（loop），并把 loop 过程中的产出记录下来供后续轮次参考改进。详细设计（SDD 层面：graph_manifest 的具体 JSON 形状、`topological_layers()`/`run_graph()` 的具体改造点、循环历史的具体持久化方式）留到本文档确认后单独产出。 +**状态**: ✅ 已实现（2026-08-08）。原「8. 开放问题」的 7 个分叉点均已拍板，记录见「8. 已确认的决策」，正文相应章节已按决策更新。详细设计见 `docs/todo/work-graph-self-planning-and-loop-back-design.md`；Part A（判断+回路引擎）与 Part B（Planner 自规划）均已实现，`python3 -m unittest discover -s src`（692 tests）+ `discover -s supervisor -p "test_*.py"`（126 tests）全绿。尚未做真实 `RUN_REAL_AGENT_E2E=1` 端到端验证（需要真实 API 额度，等待用户决定是否现在跑）。 + +## 1. 背景 + +### 1.1 现状：三条独立的、能力有限的机制 + +- **Planner 自己的文档链**（`work_graph_plan.py::_PROMPT_TEMPLATE`）：过一层真实的 LLM 规划调用，但提示词硬编码要求"必须为 brd/prd/sdd/benchmark/test_dataset 各出一个节点"（test_script 已于 2026-08-07 从这个固定清单里移除，见 `docs/todo/validator-test-script-narrative-output-issue.md`）——节点集合本质是固定的，规划这一步只是在确认依赖关系，不是真的自由决策"要不要有某个节点"。这是 2026-08-06 的一次明确设计决定（`docs/done/dynamic-work-graph-design-skill-design.md` §3 原话："不是让 Claude 完全自由决定 Planner 要生成哪些节点——Planner 那条链是'架构常量'"）。 +- **`_GENERIC_PROMPT_TEMPLATE`**（同文件，服务 Coder/Validator 的子任务图）：真正让 LLM 自由规划 `{node_id, depends_on, task_prompt}`，但目前只有 `work_graph_fork.py`（人工发起的 time-travel 调试功能）在用，没有接入任何生产路径的自动执行；且和 Planner 的固定链一样，只是一张纯 DAG，没有判断/回路概念。 +- **`work_graph_orchestrate.py::run_graph()` 的验证-重试机制**：`_verify_and_maybe_regenerate()` 只支持"某节点验证失败 → 原地重新生成同一个节点一次 → 还失败就整体 `ConsistencyCheckFailedError`"。这不是回路——失败结果不会被送回更早的节点，历史反馈也只保留最近一轮（`"; ".join(feedback)` 直接拼进 reflect prompt，不会累积多轮）。 + +### 1.2 两个此前未被充分利用的既有事实 + +- **`edge_type`（`zero_token`/`needs_judgment`）字段已经存在于 `GRAPH_MANIFEST_SCHEMA`，但从未被任何执行逻辑读取**——`topological_layers()` 只看 `depends_on`/`from-to` 决定分层，完全不管 `edge_type` 的值，是纯文档字段。 +- **`topological_layers()` 对任何环直接 `raise ValueError`**——现在的执行引擎从数据结构到算法都是严格 DAG，没有"跳回更早节点"这种控制流的容身之处。`docs/sdd/01-M-模型设计.md:60` 里 `work_graph_run.graph_manifest` 字段的文档说明其实从一开始就写了"概念上应涵盖 fan-out/fan-in 分组、loop 规则"，只是从未真正实现过。 + +### 1.3 一个必须正面回应的架构决定 + +`docs/graphlemon_architecture.md`"方案 A"明确设想过：Work Graph 内部的 fan-out/fan-in/loop/verifier 应该直接用 Claude Code 原生 Dynamic Workflow 原语（`agent()`/`parallel()`/`pipeline()`，类似 loop-until-dry）编排，而不是手撸 Python 引擎。但 `work_graph_orchestrate.py` 的实际实现故意背离了这个设想（一个节点一个独立 `claude -p` 子进程 + Python 拓扑引擎逐节点持久化状态），原因写在该模块自己的 docstring 里：REQ-06 断点续跑——原生原语跑在一次不透明的 `claude -p` 调用里，中途崩溃就会丢失全部循环历史和已完成节点状态；现在的设计换来的是"进程重启后能从任意已完成节点继续，不重跑"。 + +本需求延续这个既有背离，在 Python 引擎里新增回路能力，而不是回退去用原生原语——因为 REQ-06 是已经在生产里被真实依赖的能力（self-hosting 崩溃恢复、worktree resume），原生原语给不了这个保证。 + +## 2. 目标 + +1. `graph_manifest` 的节点能声明式地表达"怎么判断这个节点成功/失败"（机械检查引用，或另一次 LLM 判断），不再要求调用方在 `e2e_pipeline.py` 里为每种 artifact_type 手写专门的 `verify_fn` 分支。 +2. `graph_manifest` 的节点能声明式地表达"失败之后怎么办"：原地重试（现状）、跳回更早的某个指定节点重新执行（新增）、或直接判定整图失败（现状的显式化）。 +3. 触发"跳回更早节点"时，被跳回节点到失败节点之间的整条下游链要被重新执行（下游数据是基于坏输入产出的，不能继续用）；同时，这个节点每一轮被拒绝的历史（不只是最近一轮）要被完整地喂给它自己的下一次重新生成，让它能看到"过去几次分别为什么被拒绝"，而不是每次都从零开始猜。 +4. 这套回路能力必须有可配置的上限，且在进程中途崩溃重启后能正确恢复"当前是第几轮"，不能死循环也不能重置归零。 +5. 在回路执行引擎具备之后，让 Planner 自己的规划调用能真正决定"这个 goal 需要哪些节点"，而不是像现在这样被提示词硬编码死一份固定清单——这份清单本身也应该是可扩展的数据，为未来接入更多 tool/skill 节点类型留出空间（这轮不要求真的新增新类型，只要求机制本身可扩展）。 +6. **长期方向（本轮不实施，仅作为设计约束）**：用户明确的最终目标是"Planner 改造成功后，Coder、Validator、甚至未来新增的 org graph node，都需要具备同样的能力——先 plan 自己的 work graph，再按照 work graph 执行"。本轮的 Part A（judgment/on_fail 机制）设计必须是 org-agent 无关的通用能力，不能是 Planner 专属的特殊逻辑，即使这轮只把它接到 Planner 一个 org node 上（见「3. 非目标」第 1 条、「8. 已确认的决策」第 6 条）。 + +## 3. 非目标（本轮明确不做） + +1. **本轮实施范围仍只覆盖 Planner，不把 Coder/Validator 也接上自规划**。`_VALIDATOR_MANIFEST` 现在完全是硬编码的 Python 字面量，连"过一层 LLM 规划"都没有；Coder 现在是合成单节点图。用户已确认长期方向是所有 org agent（含未来新增的）都要具备同样能力（见「2. 目标」第 6 条），但落地顺序是"Planner 改造验证成功后，再依次推广"——不是本轮的实施范围，本轮只是设计时不能把机制设计成 Planner 专属。 +2. **不回退到 Claude Code 原生 Dynamic Workflow 原语**（见 1.3）。继续在 Python 层的 `work_graph_orchestrate.py` 里扩展执行引擎，保住 REQ-06 断点续跑。 +3. **不追求完全通用的图执行框架**。回路的触发方式限定为"某节点判断失败 → 跳回自己声明的 `on_fail.target_node_id`"，不支持任意节点间的条件分支、多目标跳转、或图结构在运行时被重新规划——这些是更大的能力，不在这轮范围内。 +4. **不重新设计跨 Org 的既有 loop 机制**（`org_level_loop.py`/`org_loop_route.py`：Validator FAIL/INCONCLUSIVE → 判根因 → 路由回 Planner/Coder，`loop_round` 封顶 3 轮）。本需求的回路是"同一个 Org Agent 自己的 Work Graph 内部"这一层，比跨 Org 的 loop 粒度更细，两者独立计数、不共享额度、不互相替代——内层耗尽自己的上限后照旧抛 `ConsistencyCheckFailedError`，外层机制不变地接管。 +5. **不在本需求里解决 test_script/validator_test_script 节点"叙述文字而非代码"的结构性根因**。这个节点已于 2026-08-07 因该问题禁用（`docs/todo/validator-test-script-narrative-output-issue.md`），根因是"探索后收尾习惯"，跟本需求要解决的问题（节点集合是否可以自由规划、失败能否形成回路）完全正交。用户已确认（见「8. 已确认的决策」第 7 条）：Part B 上线后，test_script 可以直接作为可选节点提供给 Planner 自由规划选择，不需要等这个结构性问题修复；该结构性问题的真正修复安排在另一份文档里，跟"给 Coder agent 加入 test_script 模块"一起解决——本需求只负责不阻塞它，不负责修它。 +6. **不修改现有 `_GENERIC_PROMPT_TEMPLATE` 路径本身**（Coder/Validator 子任务图、`work_graph_fork.py` 用的那条）。如果这轮的 judgment/on_fail 字段设计得足够通用，未来可以顺带让这条路径也受益，但不是本轮的验收范围。 + +## 4. 功能需求 + +分两部分：Part A 是执行引擎扩展（判断 + 回路能力），Part B 是 Planner 自规划节点集合。**Part B 依赖 Part A 先完成**——自由规划出的图如果没有真正的失败回路可用，只是重新引入"LLM 自由吐图结构"的脆弱性（2026-08-03 那次真实故障：LLM 漏给 `depends_on`/`edges`，`benchmark` 排到 `sdd` 前面执行，直接 `KeyError` 崩溃），却拿不到新能力。 + +### Part A：节点判断 + 失败回路执行引擎（`work_graph_orchestrate.py`） + +| # | 需求描述 | 优先级 | +|---|---|---| +| FR-A1 | `graph_manifest` 的节点应支持声明式的 `judgment` 配置（机械检查引用，或触发一次额外 LLM 判断调用），替代现在"调用方在 Python 里为每种 artifact_type 手写 `verify_fn` 分支"的模式。 | 必须 | +| FR-A2 | `graph_manifest` 的节点应支持声明式的 `on_fail` 配置，至少支持三种行为：`retry_self`（现状：原地重新生成同一节点一次）、`loop_back`（跳转到指定的更早节点重新执行）、`hard_fail`（不重试，直接判定整图失败——现状默认行为的显式化）。 | 必须 | +| FR-A3 | 触发 `loop_back` 时，不只重跑 `target_node_id` 本身，还要重跑它到触发失败的节点之间的整条下游链，因为这条链上的产出都是基于（即将被修正的）过期输入生成的。 | 必须 | +| FR-A4 | `loop_back` 的每一轮反馈（这一轮产出了什么、为什么被判定失败）必须可完整还原，且 `target_node_id` 重新执行时，累积的完整历史（不只是最近一轮）要被喂进它的提示词。**不新增专门的历史表**——复用现有 `execution_event` 日志机制（`node_start`/`node_completed` 等既有事件），每次 `loop_back` 触发/重新执行时在事件里记录 predecessor/successor 节点关联，让"这个节点被打回过几次、每次因为什么"可以直接从现有事件流重建，同时让现有的图可视化机制（`v_graph_visualizer`）能把回路自然地呈现成图上的一条边，不需要另外维护一份历史、也不需要事后做双份数据的一致性对账。 | 必须 | +| FR-A5 | 每一对 `(loop_back 目标节点, 触发回路的失败节点)` 需要独立计数，并有上限；超限时整个 `run_graph()` 调用判定失败（复用/扩展现有 `ConsistencyCheckFailedError` 语义），不允许无限循环。**上限不是写死的全局常量**——复用 `settings_config.py`（现有 `org_concurrency_config` 的同类模式：DB 存储、`get_/upsert_` 读写接口）新增一张按 `(org_node, node 类型或 node_id 模式)` 维度可配置的上限表，允许后续根据真实调用经验（哪些节点类型容易被反复打回、上限设多少更合理）动态调整，不需要改代码重新部署；节点自身也可以在 `on_fail.max_attempts` 里覆盖这个默认值。 | 必须 | +| FR-A6 | 循环计数与历史必须能扛得住 REQ-06 式的断点续跑：进程在循环中途崩溃重启后，应正确恢复"当前是第几轮、历史反馈有哪些"，不能从零重置，也不能因为状态丢失而死循环。 | 必须 | +| FR-A7 | `topological_layers()` 的静态分层只应基于"数据依赖边"（`depends_on`/`consumes`）；节点声明的 `on_fail.target_node_id`（运行时才可能触发的控制流跳转）不参与静态拓扑排序，不应被当成图里的环而报错。 | 必须 | +| FR-A8 | 不声明 `judgment`/`on_fail` 字段的既有 `graph_manifest`（例如 Coder 现在的合成单节点图）行为必须完全不变——这两个字段可选，缺省等价于现状（`judgment=none` 直接通过、`on_fail=hard_fail` 即现在的"验证失败重试一次后硬失败"）。 | 必须 | +| FR-A9 | `judgment`/`on_fail` 机制的实现（`graph_manifest` schema、`run_graph()` 的执行逻辑、FR-A5 的上限配置表）必须是 org-agent 无关的通用能力——不能依赖任何 `org_node == "planner"` 的特判。这轮只有 Planner 会真正产出带这些字段的 `graph_manifest`，但 Coder/Validator 未来接入时，不应该需要改 `work_graph_orchestrate.py` 本身，只需要它们自己的规划提示词开始产出这些字段（呼应「2. 目标」第 6 条的长期方向）。 | 必须 | + +### Part B：Planner 自规划节点集合（依赖 Part A） + +| # | 需求描述 | 优先级 | +|---|---|---| +| FR-B1 | Planner 的规划提示词从"必须产出固定清单里的每一个节点"改为：给定一份可用节点类型清单，由 LLM 自己判断这个 goal 需要哪些节点、它们的依赖关系，以及（复用 Part A）每个节点的 `judgment`/`on_fail` 配置。 | 必须 | +| FR-B2 | 可用节点类型清单本身应是可扩展的数据（而不是硬编码在提示词字符串里的固定枚举），为未来接入更多 tool/skill 节点类型留出空间；本轮不要求真的新增除现有文档类型之外的新类型。 | 应该 | +| FR-B3 | 自由规划出的 `graph_manifest` 必须经过与现有 `_normalize_graph_manifest` 同等严格的健壮性校验/兜底，防止 2026-08-03 那次真实故障重演（LLM 输出遗漏 `depends_on`/`edges`，导致执行顺序错误甚至崩溃）——节点集合变自由不代表可以降低这层鲁棒性要求。 | 必须 | +| FR-B4 | **brd/prd/sdd/benchmark/test_dataset（以及 test_script，一旦被规划出来）全部由 LLM 自由规划，不保留任何代码级的固定顺序/固定节点集合兜底**——已确认（「8. 已确认的决策」第 1 条）：完全推翻 `docs/done/dynamic-work-graph-design-skill-design.md` §3"Planner 链是架构常量"的决定。FR-B3 的健壮性校验兜底（防止 LLM 漏给 `depends_on`/`edges`）因此变得更重要：没有任何硬编码的顺序可以兜底，健壮性完全依赖 FR-B3。 | 必须 | + +## 5. 验收标准 + +1. 构造一个 `graph_manifest`：节点 A 产出，节点 B 依赖 A、声明 `judgment` 机械检查、`on_fail=loop_back` 指向 A；用 mock 让 B 连续两轮判断失败、第三轮通过。断言：A 被重新执行了两次（不是原地重试 B），且 A 第二次、第三次重新执行时收到的提示词里包含前面所有轮次的失败反馈（不只是最近一轮）。 +2. 同上场景，把 B 的失败改成"每一轮都失败"，断言在达到可配置上限后，`run_graph()` 判定整图失败并明确抛出（而不是无限循环挂起），且失败前 A 已经产出的内容仍按现有"部分成功节点落盘"机制（`_write_completed_node_artifacts`）被正确保留。 +3. 模拟"回路进行到第 2 轮时进程中断重启"，断言恢复执行后能正确识别"已经是第 2 轮"并继续，而不是把计数器归零重新从第 1 轮开始，也不是把已经持久化的历史反馈丢掉。 +4. 一个声明了 `on_fail.target_node_id` 指向更早节点的 `graph_manifest`，`topological_layers()`/图校验逻辑不应把它当作静态环拒绝（不抛 `ValueError`）。 +5. 一个完全不使用 `judgment`/`on_fail` 字段的既有 `graph_manifest`（例如 Coder 当前用的合成单节点图），跑一遍 `run_graph()`，行为与改造前逐字节一致——现有 `src/test_work_graph_orchestrate.py`/`src/test_e2e_pipeline.py` 里未显式测试新字段的用例，不修改断言即可继续通过。 +6. （Part B）给 Planner 一个明显不需要 test_dataset 的 goal（例如纯配置项调整），断言自规划产出的 `graph_manifest` 里确实没有 test_dataset 节点，且整条链路（Coder 拿到的 `test_dataset_content` 走 `"N/A: not provided"` 兜底）不因为节点缺失而报错——复用现有的 `"test_dataset" in artifact_ids` 容错逻辑，不需要新代码。 +7. （Part B）人为构造一个自由规划回复"遗漏 `depends_on`"的场景（模拟 2026-08-03 那次真实故障的输入），断言健壮性兜底仍能纠正执行顺序，不崩溃。 +8. （Part B）给 Planner 一个明显需要机械化验收脚本的 goal，可用节点类型清单里包含 test_script，断言 Planner 能够（不是必须，是"能够"）自由规划出一个 test_script 节点——确认 test_script 已按「8. 已确认的决策」第 7 条重新开放为可选节点，不再被清单排除。 +9. （FR-A5）验证上限配置确实按 `(org_node, node 类型)` 生效：给同一个 org_node 下两种不同 node 类型分别配置不同上限，构造两个各自持续失败的 loop_back 场景，断言各自在自己配置的上限触发失败，不是共用同一个全局值；再验证节点自身 `on_fail.max_attempts` 能覆盖配置表里的默认值。 +10. （FR-A4）验证 loop_back 历史确实通过 `execution_event` 而非新表重建：触发一次多轮 loop_back 后，仅查询 `execution_event` 表（不查询任何新增表）即可还原"这个节点被打回几次、每次的 predecessor/successor 是谁、每次的反馈内容"。 + +## 6. 约束与假设 + +- 复用 REQ-06 已经建立的"`work_node_execution.output_payload` 是节点成功后的权威持久化数据"这一既有约定；回路历史**不新增专门的表**，完整复用 `execution_event` 现有机制（见 FR-A4），必须遵循"重启后可完整恢复"这一约束，不引入新的、易丢失的临时状态。 +- 复用 `org_loop_route.py::_LOOP_LIMIT` 的既有模式（"计数 + 达到上限即转 `blocked_pending_human`"）作为 Part A 回路上限触发后果的参照，但两者是独立的计数器，不共享同一个 `task.loop_round` 字段；上限的具体数值来源改为复用 `settings_config.py` 的 DB 配置模式（见 FR-A5），不是抄一个固定常量。 +- `judgment.kind="llm"` 复用 `_check_prd_consistency` 现有模式：额外发起一次 `agent()` 调用（`agentType='planner'`），self-target 场景下走独立 worktree 隔离，避免重新设计一套判断机制。 +- Part A 完成后，Coder 现在的合成单节点图、`_GENERIC_PROMPT_TEMPLATE` 路径都不强制迁移使用新字段——新字段是可选能力，不是所有 `graph_manifest` 生产者都必须立刻采用；但机制本身必须 org-agent 无关（见 FR-A9），为后续推广铺路。 + +## 7. 风险与已知权衡 + +- **用户已知情并接受：Part B 上线后 test_script 类节点可能被自由规划回来，narrative-output 问题可能重现**。`docs/todo/validator-test-script-narrative-output-issue.md` 记录的根因是"探索后收尾习惯"，跟本需求解决的问题（节点集合能不能自由规划、失败能否形成回路）完全正交，本需求不修它。用户决策（「8. 已确认的决策」第 7 条）：不等结构性修复，真正的修复安排在"给 Coder agent 加入 test_script 模块"那份文档里一起解决——本需求上线后，如果自举/真实任务里又复现了叙述文字问题，这是已知的、被接受的风险，不是本需求的回归。 +- **回路能力本身可能被滥用成隐藏的高成本重试**：一个设计不当的 `judgment`（例如过于严格的 LLM 判断标准）配合 `loop_back`，可能导致某个节点反复被打回、消耗大量 token 才勉强通过上限。FR-A5 的按 `(org_node, node 类型)` 可配置上限缓解了"一刀切"的问题，但初始默认值仍需要设计阶段给出具体理由，不能只是"抄一个数字"；上线后应该根据真实调用数据回头调整（这正是选择"DB 配置表"而不是"硬编码常量"的原因）。 +- **Part B 是对已有架构决定的正式推翻**，不是渐进式增强。`docs/done/dynamic-work-graph-design-skill-design.md` §3 需要在 Part B 实施后同步更新，避免文档和代码再次出现本 session 里多次遇到的"文档记录的意图与实际代码不一致"的问题。 +- **Part A + Part B 一并实施（已确认，见「8. 已确认的决策」第 5 条），没有中间的"仅 Part A 先跑一段时间验证稳定性"的缓冲阶段**——意味着 Part A 的回路引擎第一次被真实使用，就是在"节点集合本身也是自由规划、不受固定链条约束"这个更不可预测的场景下，两个新变量同时引入，出问题时定位"是回路引擎的 bug 还是自由规划出了一个奇怪的图结构"会比分阶段更难。这是用户已知情的权衡，不是本需求遗漏。 + +## 8. 已确认的决策（2026-08-08） + +1. **Part B 的推翻力度**：全部自由规划。brd/prd/sdd/benchmark/test_dataset（以及重新开放的 test_script）不保留任何代码级固定顺序/固定节点集合兜底，完全推翻 `docs/done/dynamic-work-graph-design-skill-design.md` §3 的既有决定（对应 FR-B4）。健壮性完全依赖 FR-B3 的校验兜底。 +2. **回路上限**：不设单一全局默认值，按 `(org_node, node 类型)` 可配置，复用 `settings_config.py` 的 DB 配置模式，根据实际调用经验动态调整（对应 FR-A5）。 +3. **`judgment.kind="llm"` 的实现**：复用现有 `_check_prd_consistency` 模式（额外发起一次 `agent()` 调用，`agentType='planner'`，self-target 场景走独立 worktree）。 +4. **回路历史持久化**：不新增专门的表，完整复用 `execution_event` 现有日志机制，记录 predecessor/successor 节点关联，天然可被现有图可视化机制呈现为有向图上的边（对应 FR-A4）。 +5. **落地顺序**：Part A、Part B 一起做，不分阶段（见「7. 风险」最后一条，这是已知权衡，不是遗漏）。 +6. **Validator/Coder 是否一起做**：本轮仍只落地 Planner，但 Part A 的机制设计必须 org-agent 无关（FR-A9）——用户明确的长期方向是 Planner 验证成功后，Coder、Validator、乃至未来新增的 org graph node 都要具备"先 plan 自己的 work graph，再按图执行"的同一能力（见「2. 目标」第 6 条）。 +7. **test_script 是否要等结构性修复**：不需要等。Part B 上线后，test_script 直接作为可选节点开放给 Planner 自由规划；narrative-output 问题的真正结构性修复安排在"给 Coder agent 加入 test_script 模块"的另一份文档里一起解决，不是本需求的阻塞项（见「7. 风险」第一条）。 diff --git a/src/backend_server.py b/src/backend_server.py index ca0bd35..2658818 100644 --- a/src/backend_server.py +++ b/src/backend_server.py @@ -1036,6 +1036,8 @@ def _ensure_schema(conn): # startup). Run all three every time, not just on first-ever creation. schema.migrate_expand_artifact_types(conn) schema.migrate_relax_org_node_constraints(conn) # also runs migrate_add_work_graph_fork_columns internally + schema.migrate_add_work_graph_loop_limit_config(conn) + schema.migrate_add_loop_back_event_type(conn) class _Server(ThreadingHTTPServer): diff --git a/src/e2e_pipeline.py b/src/e2e_pipeline.py index 551bc5e..6ac7fc7 100644 --- a/src/e2e_pipeline.py +++ b/src/e2e_pipeline.py @@ -208,6 +208,10 @@ _VALIDATOR_DOC_PROMPTS = { ), } +# validator_test_script node temporarily disabled (2026-08-07, same +# stopgap as work_graph_plan._PROMPT_TEMPLATE -- see its comment for the +# full rationale/upgrade path): 6/6 real repro rate for narrative-instead- +# of-code output, docs/todo/validator-test-script-narrative-output-issue.md. _VALIDATOR_MANIFEST = { "nodes": [ {"node_id": "validator_benchmark", "artifact_type": "validator_benchmark", "depends_on": []}, @@ -215,15 +219,6 @@ _VALIDATOR_MANIFEST = { "node_id": "validator_test_dataset", "artifact_type": "validator_test_dataset", "depends_on": ["validator_benchmark"], }, - { - # depends on BOTH -- its own prompt needs - # {validator_benchmark_content} directly, not just what - # survives into validator_test_dataset_content (same reasoning - # as the Planner's own test_script node, see - # work_graph_plan._normalize_graph_manifest). - "node_id": "validator_test_script", "artifact_type": "validator_test_script", - "depends_on": ["validator_test_dataset", "validator_benchmark"], - }, ], "edges": [], } @@ -240,6 +235,24 @@ _VALIDATOR_VERDICT_PROMPT = ( ) +class _MissingContentDefault(dict): + """2026-08-08 (found via a real self-planned run, task involving a + src/work_node_run.py bug fix): `_DOC_PROMPTS` entries were all written + for the old fixed brd->prd->sdd->benchmark->... chain and hardcode + placeholders like {prd_content} assuming that upstream node always + exists. Work Graph self-planning (FR-B4 "全部自由规划") legitimately + lets the Planner skip nodes that don't apply -- this real run's SDD + node correctly had no PRD node at all (a small bug-fix goal doesn't + need one), so `_DOC_PROMPTS["sdd"]`'s `{prd_content}` placeholder had + nothing to fill it and `.format()` raised a bare KeyError, landing the + task on `blocked_pending_human` with the unhelpful generic + 'execution_error' reason. `str.format_map` with this as the mapping + substitutes a clear placeholder for any content key the self-planned + graph didn't actually produce, instead of crashing.""" + def __missing__(self, key): + return "N/A (no corresponding upstream node was included in this Work Graph)" + + def _planner_prompt_builder(goal_text, node_artifact_types): """node_artifact_types: dict[node_id] -> artifact_type, read off the real graph_manifest before running it (build_prompt only ever sees one @@ -247,13 +260,13 @@ def _planner_prompt_builder(goal_text, node_artifact_types): def build_prompt(node, outputs): node_id = node["node_id"] artifact_type = node_artifact_types.get(node_id, node_id) - content_kwargs = {} + content_kwargs = {"goal_text": goal_text} for dep_id in (node.get("depends_on") or []): dep_output = outputs.get(dep_id) if dep_output is not None: dep_type = node_artifact_types.get(dep_id, dep_id) content_kwargs[dep_type + "_content"] = dep_output["content"] - return _DOC_PROMPTS[artifact_type].format(goal_text=goal_text, **content_kwargs) + return _DOC_PROMPTS[artifact_type].format_map(_MissingContentDefault(content_kwargs)) return build_prompt @@ -558,6 +571,20 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): re-raised unchanged after salvaging -- this does not change whether or when a task ends up blocked_pending_human, only what's left behind for a human's Resume to build on when it does. + + 2026-08-08 (Work Graph self-planning + loop-back, FR-B1): `verify_fn` + is no longer `_combined_planner_verify_fn` -- each self-planned node + now carries its own `judgment` field (work_graph_plan.py's + `_PLANNER_NODE_CATALOG` suggests one per node type, the Planner can + reuse or override it), which `run_graph()` resolves itself per node. + `_combined_planner_verify_fn`/`_prd_consistency_verify_fn`/ + `_test_script_verify_fn` are NOT deleted -- `validate_implementation_ + with_own_graph()` (Validator's still-hardcoded manifest, unaffected by + this round) continues to use them (FR-A8). `mechanical_check_registry` + is still passed unconditionally: a self-planned node declaring + `judgment.kind == "mechanical"` (e.g. reusing the catalog's suggested + `check_script_compiles` for a test_script node) needs it registered, + same as before. """ plan_result = work_graph_plan.plan_work_graph( conn, "planner", task_id, goal_text, target_folder_path, timeout=timeout @@ -571,7 +598,7 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): work_graph_orchestrate.run_graph( conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, - verify_fn=_combined_planner_verify_fn(node_artifact_types, target_folder_path), + mechanical_check_registry={"check_script_compiles": check_script_compiles}, ) except Exception as e: run_graph_error = e @@ -792,10 +819,16 @@ def validate_implementation_with_own_graph( if run_graph_error is not None: raise run_graph_error - script_content = outputs["validator_test_script"]["content"] - script_execution = run_test_script_for_real( - script_content, target_folder_path, task_id, "validator_test_script", timeout=timeout, - ) + if "validator_test_script" in outputs: + script_content = outputs["validator_test_script"]["content"] + script_execution = run_test_script_for_real( + script_content, target_folder_path, task_id, "validator_test_script", timeout=timeout, + ) + else: + script_execution = { + "ran": False, "exit_code": None, "stdout": "", "stderr": "", + "note": "N/A: validator_test_script node is currently disabled (see _VALIDATOR_MANIFEST)", + } response = org_node_trigger.trigger_org_node( "validator", diff --git a/src/schema.py b/src/schema.py index 4270e0c..f87d6dc 100644 --- a/src/schema.py +++ b/src/schema.py @@ -2,7 +2,8 @@ U-001 scope: the 15 platform entities from docs/sdd/01-M-模型设计.md §1.0-1.14 (PH0-PH4). `deployment`/`deploy_health_signal` (§1.15/§1.16, added here in U-034, -2026-08-02) are PH5-only -- 17 tables total as of PH5. +2026-08-02) are PH5-only. `work_graph_loop_limit_config` (§1.11.1, 2026-08-08, +Work Graph self-planning + loop-back) added post-PH5 -- 18 tables total. Interpretive conventions used below, where 01-M leaves the physical type implicit: - Booleans are stored as INTEGER CHECK(x IN (0, 1)) -- sqlite3 has no native bool type. @@ -46,6 +47,7 @@ TABLE_NAMES = ( "agent_profile", "budget_guardrail_config", "org_concurrency_config", + "work_graph_loop_limit_config", "execution_event", "execution_log_file", "log_retention_config", @@ -262,7 +264,7 @@ CREATE TABLE execution_event ( event_type TEXT NOT NULL CHECK (event_type IN ( 'task_status_changed', 'node_status_changed', 'artifact_created', 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', - 'log_appended', 'quota_wait_started' + 'log_appended', 'quota_wait_started', 'loop_back_triggered' )), payload TEXT NOT NULL, created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP @@ -271,6 +273,15 @@ CREATE TABLE execution_event ( CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); +-- 1.11.1 work_graph_loop_limit_config (Work Graph self-planning + loop-back, +-- docs/todo/work-graph-self-planning-and-loop-back-design.md §6) +CREATE TABLE work_graph_loop_limit_config ( + config_id TEXT PRIMARY KEY, + max_loop_attempts INTEGER NOT NULL, + updated_by TEXT, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP +); + -- 1.13 execution_log_file CREATE TABLE execution_log_file ( log_file_id TEXT PRIMARY KEY, @@ -581,3 +592,57 @@ def migrate_relax_org_node_constraints(conn): ) finally: conn.execute("PRAGMA foreign_keys = ON") + + +def migrate_add_work_graph_loop_limit_config(conn): + """2026-08-08 (Work Graph self-planning + loop-back, + docs/todo/work-graph-self-planning-and-loop-back-design.md §6): adds + work_graph_loop_limit_config on an ALREADY-POPULATED database. Plain + `CREATE TABLE IF NOT EXISTS` -- a brand-new table, no CHECK constraint + to rebuild around, unlike migrate_expand_artifact_types. Idempotent and + safe to call unconditionally, including against a freshly + create_schema()'d database (which already has it).""" + conn.execute( + "CREATE TABLE IF NOT EXISTS work_graph_loop_limit_config (" + "config_id TEXT PRIMARY KEY, max_loop_attempts INTEGER NOT NULL, " + "updated_by TEXT, updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP)" + ) + conn.commit() + + +def migrate_add_loop_back_event_type(conn): + """2026-08-08 (Work Graph self-planning + loop-back, + docs/todo/work-graph-self-planning-and-loop-back-design.md §7): adds + 'loop_back_triggered' to execution_event.event_type's CHECK enum on an + ALREADY-POPULATED database -- same create-copy-drop-rename dance as + migrate_expand_artifact_types (SQLite has no ALTER-based constraint + migration). Idempotent and safe to call unconditionally, including + against a freshly create_schema()'d database (which already has the + expanded constraint).""" + row = conn.execute("SELECT sql FROM sqlite_master WHERE type='table' AND name='execution_event'").fetchone() + if row is None or "'loop_back_triggered'" in row[0]: + return + conn.executescript( + """ + CREATE TABLE execution_event_new ( + event_id TEXT PRIMARY KEY, + project_id TEXT NOT NULL REFERENCES project(project_id), + task_id TEXT REFERENCES task(task_id), + run_id TEXT REFERENCES work_graph_run(run_id), + node_exec_id TEXT REFERENCES work_node_execution(node_exec_id), + event_type TEXT NOT NULL CHECK (event_type IN ( + 'task_status_changed', 'node_status_changed', 'artifact_created', + 'bug_recorded', 'loop_routed', 'budget_updated', 'skill_iterated', + 'log_appended', 'quota_wait_started', 'loop_back_triggered' + )), + payload TEXT NOT NULL, + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + INSERT INTO execution_event_new SELECT * FROM execution_event; + DROP TABLE execution_event; + ALTER TABLE execution_event_new RENAME TO execution_event; + CREATE INDEX idx_execution_event_project_created ON execution_event(project_id, created_at); + CREATE INDEX idx_execution_event_task_created ON execution_event(task_id, created_at); + """ + ) + conn.commit() diff --git a/src/settings_config.py b/src/settings_config.py index 53c1f12..b06ebcc 100644 --- a/src/settings_config.py +++ b/src/settings_config.py @@ -45,6 +45,44 @@ def upsert_concurrency_config(conn, max_concurrent_goals, updated_by, config_id= return get_concurrency_config(conn, config_id) +def get_loop_limit_config(conn, config_id): + """Work Graph self-planning + loop-back (docs/todo/work-graph-self- + planning-and-loop-back-design.md §6): no single 'default' config_id -- + callers look up "{org_node}:{node_type}" then "{org_node}:*" then + "default", falling back to a hardcoded constant if even "default" is + unseeded (same pattern scheduler.py already uses for + get_concurrency_config: `config["x"] if config else `).""" + row = conn.execute( + "SELECT config_id, max_loop_attempts, updated_at FROM work_graph_loop_limit_config WHERE config_id = ?", + (config_id,), + ).fetchone() + if row is None: + return None + return {"config_id": row[0], "max_loop_attempts": row[1], "updated_at": row[2]} + + +def upsert_loop_limit_config(conn, config_id, max_loop_attempts, updated_by): + if max_loop_attempts < 1: + raise InvalidParamError("max_loop_attempts must be >= 1, got {!r}".format(max_loop_attempts)) + + exists = conn.execute( + "SELECT 1 FROM work_graph_loop_limit_config WHERE config_id = ?", (config_id,) + ).fetchone() is not None + if exists: + conn.execute( + "UPDATE work_graph_loop_limit_config SET max_loop_attempts = ?, updated_by = ?, " + "updated_at = CURRENT_TIMESTAMP WHERE config_id = ?", + (max_loop_attempts, updated_by, config_id), + ) + else: + conn.execute( + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) VALUES (?, ?, ?)", + (config_id, max_loop_attempts, updated_by), + ) + conn.commit() + return get_loop_limit_config(conn, config_id) + + def get_log_retention_config(conn, config_id="default"): row = conn.execute( "SELECT config_id, retention_days, max_total_size_mb, updated_at FROM log_retention_config " diff --git a/src/test_e2e_pipeline.py b/src/test_e2e_pipeline.py index 2ff4d79..1ecd157 100644 --- a/src/test_e2e_pipeline.py +++ b/src/test_e2e_pipeline.py @@ -22,6 +22,7 @@ or work_graph_orchestrate's own mechanics (test_work_graph_orchestrate.py). Run: python3 -m unittest src.test_e2e_pipeline -v """ +import contextlib import json import os import subprocess @@ -37,6 +38,13 @@ _FAKE_DOCS = { "test_script": "assert True # fake but real, compilable script", } +# test_script disabled 2026-08-07 (docs/todo/validator-test-script- +# narrative-output-issue.md) -- _PROMPT_TEMPLATE no longer asks for it, so +# the realistic fake planning response matches that. Tests that still need +# to exercise a test_script node (e.g. the partial-failure salvage +# mechanism, which is generic and doesn't care whether test_script is +# currently enabled by default) build their own local manifest instead of +# using this shared fixture. _FAKE_GRAPH_MANIFEST = { "nodes": [ {"node_id": "brd", "artifact_type": "brd", "depends_on": []}, @@ -44,14 +52,44 @@ _FAKE_GRAPH_MANIFEST = { {"node_id": "sdd", "artifact_type": "sdd", "depends_on": ["prd"]}, {"node_id": "benchmark", "artifact_type": "benchmark", "depends_on": ["sdd"]}, {"node_id": "test_dataset", "artifact_type": "test_dataset", "depends_on": ["benchmark"]}, - {"node_id": "test_script", "artifact_type": "test_script", "depends_on": ["test_dataset"]}, ], "edges": [ {"from": "brd", "to": "prd", "edge_type": "needs_judgment"}, {"from": "prd", "to": "sdd", "edge_type": "needs_judgment"}, {"from": "sdd", "to": "benchmark", "edge_type": "needs_judgment"}, {"from": "benchmark", "to": "test_dataset", "edge_type": "needs_judgment"}, + ], +} + +# Only used by tests that need a test_script node present despite it being +# disabled by default (see _FAKE_GRAPH_MANIFEST's comment above). +_FAKE_GRAPH_MANIFEST_WITH_TEST_SCRIPT = { + "nodes": _FAKE_GRAPH_MANIFEST["nodes"] + [ + # depends_on both -- _DOC_PROMPTS["test_script"] needs + # {benchmark_content} directly, not just what survives into + # test_dataset_content. work_graph_plan._normalize_graph_manifest + # used to auto-inject this second dependency for the canonical + # chain's test_script node; that logic was removed along with + # test_script from _CANONICAL_DOC_CHAIN (2026-08-07), so this fixture + # sets it explicitly instead of relying on any auto-injection. + # 2026-08-08: `judgment` is required now -- plan_documents() no + # longer passes an external verify_fn at all (Work Graph + # self-planning + loop-back, FR-B1); a node only gets checked if it + # declares its own judgment, same as a real self-planned node would + # by reusing work_graph_plan._PLANNER_NODE_CATALOG's suggestion. + { + "node_id": "test_script", "artifact_type": "test_script", + "depends_on": ["test_dataset", "benchmark"], + "judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}, + # max_attempts=1 -- "1 retry allowed" (matches the legacy exactly- + # one-retry default): exactly 2 real executions before exceeding, + # matching this fixture's tests providing exactly 2 mock responses. + "on_fail": {"action": "retry_self", "max_attempts": 1}, + }, + ], + "edges": _FAKE_GRAPH_MANIFEST["edges"] + [ {"from": "test_dataset", "to": "test_script", "edge_type": "needs_judgment"}, + {"from": "benchmark", "to": "test_script", "edge_type": "needs_judgment"}, ], } @@ -61,6 +99,14 @@ def _fake_plan_subprocess_run(*args, **kwargs): return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") +def _fake_plan_subprocess_run_with_test_script(*args, **kwargs): + payload = { + "is_error": False, + "result": "```json\n" + json.dumps(_FAKE_GRAPH_MANIFEST_WITH_TEST_SCRIPT) + "\n```", + } + return subprocess.CompletedProcess(args=["claude"], returncode=0, stdout=json.dumps(payload), stderr="") + + def _fake_ready_trigger(org_node, prompt, target_folder_path, timeout=300): assert prompt == e2e_pipeline._READY_PROMPT return "Ready." @@ -80,7 +126,7 @@ class PipelineTestBase(unittest.TestCase): class PlanDocumentsTest(PipelineTestBase): - def test_produces_six_artifacts_with_correct_lineage(self): + def test_produces_five_artifacts_with_correct_lineage(self): self.conn.execute( "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " "VALUES ('task-1', (SELECT project_id FROM project), '实现贪吃蛇游戏', 'goal', " @@ -95,12 +141,12 @@ class PlanDocumentsTest(PipelineTestBase): {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, - {"content": _FAKE_DOCS["test_dataset"]}, {"content": _FAKE_DOCS["test_script"]}, + {"content": _FAKE_DOCS["test_dataset"]}, ] plan_result = e2e_pipeline.plan_documents(self.conn, "task-1", "实现贪吃蛇游戏", self.tmp.name) artifact_ids = plan_result["artifact_ids"] - self.assertEqual(set(artifact_ids), {"brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"}) + self.assertEqual(set(artifact_ids), {"brd", "prd", "sdd", "benchmark", "test_dataset"}) prd_deps = json.loads( self.conn.execute( "SELECT depends_on_artifact_ids FROM artifact WHERE artifact_id = ?", (artifact_ids["prd"],) @@ -127,6 +173,12 @@ class PlanDocumentsTest(PipelineTestBase): # still propagate (failure semantics unchanged), but BRD/PRD/SDD/ # Benchmark/TestDataset -- all genuinely produced and accepted -- # must be persisted as artifacts, not silently discarded. + # + # test_script itself is disabled by default as of the same date + # (docs/todo/validator-test-script-narrative-output-issue.md) -- this + # test uses _fake_plan_subprocess_run_with_test_script to keep + # exercising the salvage mechanism itself (generic, not specific to + # test_script) rather than losing this coverage entirely. self.conn.execute( "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " "VALUES ('task-1', (SELECT project_id FROM project), 'goal', 'goal', " @@ -134,23 +186,23 @@ class PlanDocumentsTest(PipelineTestBase): ) self.conn.commit() - # _fake_plan_subprocess_run ignores its args and always returns a - # canned success payload -- fine for the "claude" planning call it's - # meant for, but `patch("src.work_graph_plan.subprocess.run", ...)` - # patches the ONE global `subprocess` module every other importer - # shares too (work_graph_plan does `import subprocess`, not - # `from subprocess import run`), so it would ALSO silently hijack - # check_script_compiles's real `py_compile`/`node --check` calls, - # making them always report success regardless of actual content -- - # exactly defeating the failure this test needs to trigger. Only - # intercept the "claude" planning call; let everything else - # (including this test's real compile checks) run for real. + # _fake_plan_subprocess_run_with_test_script ignores its args and + # always returns a canned success payload -- fine for the "claude" + # planning call it's meant for, but `patch("src.work_graph_plan. + # subprocess.run", ...)` patches the ONE global `subprocess` module + # every other importer shares too (work_graph_plan does `import + # subprocess`, not `from subprocess import run`), so it would ALSO + # silently hijack check_script_compiles's real `py_compile`/`node + # --check` calls, making them always report success regardless of + # actual content -- exactly defeating the failure this test needs to + # trigger. Only intercept the "claude" planning call; let everything + # else (including this test's real compile checks) run for real. real_subprocess_run = subprocess.run def _discriminating_fake(*args, **kwargs): cmd = args[0] if args else kwargs.get("args") if cmd and cmd[0] == "claude": - return _fake_plan_subprocess_run(*args, **kwargs) + return _fake_plan_subprocess_run_with_test_script(*args, **kwargs) return real_subprocess_run(*args, **kwargs) with patch("src.work_graph_plan.subprocess.run", side_effect=_discriminating_fake), \ @@ -184,20 +236,19 @@ class RunEndToEndTest(PipelineTestBase): {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, - {"content": _FAKE_DOCS["test_dataset"]}, {"content": _FAKE_DOCS["test_script"]}, + {"content": _FAKE_DOCS["test_dataset"]}, {"response": "Implemented. Ran `pytest`: 3 passed."}, - # Validator's OWN independent benchmark/dataset/script (2026-08-06) -- + # Validator's OWN independent benchmark/dataset (2026-08-06) -- # deliberately different content from the Planner's, to also prove # nothing here accidentally reuses the Planner's copies. {"content": "# Validator's own Benchmark\nfake"}, {"content": "N/A: validator judged no dataset needed either"}, - {"content": "assert True # validator's own real, compilable script"}, ] result = e2e_pipeline.run_end_to_end(self.conn, self.tmp.name, "实现贪吃蛇游戏") self.assertEqual(result["verdict"]["verdict"], "PASS") self.assertEqual( - set(result["doc_artifact_ids"]), {"brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"} + set(result["doc_artifact_ids"]), {"brd", "prd", "sdd", "benchmark", "test_dataset"} ) self.assertEqual(set(result["code_artifact_ids"]), {"code_diff", "test_report"}) @@ -209,9 +260,10 @@ class RunEndToEndTest(PipelineTestBase): all_artifacts = self.conn.execute( "SELECT COUNT(*) FROM artifact WHERE task_id = ?", (result["task_id"],) ).fetchone()[0] - # brd, prd, sdd, benchmark, test_dataset, test_script, code_diff, test_report, - # validator_benchmark, validator_test_dataset, validator_test_script - self.assertEqual(all_artifacts, 11) + # brd, prd, sdd, benchmark, test_dataset, code_diff, test_report, + # validator_benchmark, validator_test_dataset + # (test_script/validator_test_script disabled, see _VALIDATOR_MANIFEST) + self.assertEqual(all_artifacts, 9) # planner, coder, AND validator (2026-08-06: validator now has its own # real work_graph_run too, not a flat direct call) each got their own @@ -338,6 +390,33 @@ class TestScriptVerifyFnTest(unittest.TestCase): self.assertTrue(ok) +class PlannerPromptBuilderTest(unittest.TestCase): + """2026-08-08: found via a real self-planned run whose SDD node had no + PRD node at all (a legitimate Work Graph self-planning choice, FR-B4 + "全部自由规划") -- _DOC_PROMPTS["sdd"] hardcodes a {prd_content} + placeholder left over from the old fixed chain, and plain .format() + raised a bare KeyError, landing the task on blocked_pending_human with + an unhelpful generic 'execution_error' reason instead of a real result.""" + + def test_missing_upstream_dependency_content_does_not_raise(self): + node_artifact_types = {"sdd": "sdd"} + build_prompt = e2e_pipeline._planner_prompt_builder("fix a bug", node_artifact_types) + # sdd's depends_on is empty -- no prd node was ever planned, so + # {prd_content} has nothing to fill it. Must not raise KeyError. + prompt = build_prompt({"node_id": "sdd", "depends_on": []}, {}) + self.assertIn("N/A (no corresponding upstream node was included in this Work Graph)", prompt) + self.assertIn("fix a bug", prompt) + + def test_present_upstream_dependency_content_is_used_normally(self): + node_artifact_types = {"prd": "prd", "sdd": "sdd"} + build_prompt = e2e_pipeline._planner_prompt_builder("goal", node_artifact_types) + prompt = build_prompt( + {"node_id": "sdd", "depends_on": ["prd"]}, {"prd": {"content": "real prd text"}} + ) + self.assertIn("real prd text", prompt) + self.assertNotIn("N/A (no corresponding", prompt) + + class CombinedPlannerVerifyFnTest(unittest.TestCase): """Dispatches to the right per-node-type check -- both real checks (PRD consistency and script-compiles) are exercised elsewhere in @@ -494,6 +573,28 @@ class RunTestScriptForRealTest(unittest.TestCase): self.assertTrue(os.path.exists(expected)) +# validator_test_script disabled by default 2026-08-07 (see +# e2e_pipeline._VALIDATOR_MANIFEST's own comment) -- tests below that need +# to exercise the hard-gate/salvage mechanisms involving that node patch +# _VALIDATOR_MANIFEST back to this 3-node shape for their own duration, so +# that real code (still in the codebase, just not planned by default) stays +# covered instead of silently rotting. +_VALIDATOR_MANIFEST_WITH_TEST_SCRIPT = { + "nodes": [ + {"node_id": "validator_benchmark", "artifact_type": "validator_benchmark", "depends_on": []}, + { + "node_id": "validator_test_dataset", "artifact_type": "validator_test_dataset", + "depends_on": ["validator_benchmark"], + }, + { + "node_id": "validator_test_script", "artifact_type": "validator_test_script", + "depends_on": ["validator_test_dataset", "validator_benchmark"], + }, + ], + "edges": [], +} + + class ValidateImplementationWithOwnGraphTest(unittest.TestCase): """2026-08-06: the safety-critical piece -- a real non-zero exit from Validator's own script must force FAIL no matter what the LLM's verdict @@ -515,21 +616,28 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): def tearDown(self): self.conn.close() - def _run(self, script_content, verdict_line): - with patch.object(work_graph_execute, "write_workflow_script"), \ - patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ - patch.object(org_node_trigger, "trigger_org_node", return_value=verdict_line): - mock_execute.side_effect = [ - {"content": "# Validator's own Benchmark"}, - {"content": "N/A: no dataset needed"}, - {"content": script_content}, - ] + def _run(self, script_content, verdict_line, with_test_script=False): + mock_responses = [ + {"content": "# Validator's own Benchmark"}, + {"content": "N/A: no dataset needed"}, + ] + if with_test_script: + mock_responses.append({"content": script_content}) + with contextlib.ExitStack() as stack: + stack.enter_context(patch.object(work_graph_execute, "write_workflow_script")) + mock_execute = stack.enter_context(patch.object(work_graph_execute, "execute_work_graph")) + stack.enter_context(patch.object(org_node_trigger, "trigger_org_node", return_value=verdict_line)) + if with_test_script: + stack.enter_context( + patch.object(e2e_pipeline, "_VALIDATOR_MANIFEST", _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT) + ) + mock_execute.side_effect = mock_responses return e2e_pipeline.validate_implementation_with_own_graph( self.conn, "task-1", "sdd content", "prd content", "goal text", self.tmp.name, ) def test_llm_says_pass_and_script_really_passes_stays_pass(self): - result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") + result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good", with_test_script=True) self.assertEqual(result["verdict"], "PASS") self.assertTrue(result["script_execution"]["ran"]) self.assertEqual(result["script_execution"]["exit_code"], 0) @@ -538,7 +646,9 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): # the exact case the user's hard-gate rule exists for: a script # that objectively fails must not be argued away by the LLM's own # verdict line. - result = self._run("import sys\nsys.exit(1)\n", "VERDICT: PASS\nlooks good to me") + result = self._run( + "import sys\nsys.exit(1)\n", "VERDICT: PASS\nlooks good to me", with_test_script=True + ) self.assertEqual(result["verdict"], "FAIL") def test_llm_says_fail_and_script_fails_stays_fail(self): @@ -550,8 +660,21 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): self.assertEqual(result["verdict"], "INCONCLUSIVE") self.assertFalse(result["script_execution"]["ran"]) - def test_writes_three_validator_artifacts_with_lineage(self): + def test_writes_two_validator_artifacts_with_lineage(self): + # test_script disabled by default -- see _VALIDATOR_MANIFEST_WITH_ + # TEST_SCRIPT variant below for the 3-artifact/script-lineage case. result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") + self.assertEqual(set(result["artifact_ids"]), {"validator_benchmark", "validator_test_dataset"}) + dataset_deps = json.loads( + self.conn.execute( + "SELECT depends_on_artifact_ids FROM artifact WHERE artifact_id = ?", + (result["artifact_ids"]["validator_test_dataset"],), + ).fetchone()[0] + ) + self.assertIn(result["artifact_ids"]["validator_benchmark"], dataset_deps) + + def test_writes_three_validator_artifacts_with_lineage_when_test_script_re_enabled(self): + result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good", with_test_script=True) self.assertEqual( set(result["artifact_ids"]), {"validator_benchmark", "validator_test_dataset", "validator_test_script"}, @@ -619,8 +742,12 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): # -- ConsistencyCheckFailedError must still propagate, but # validator_benchmark/validator_test_dataset (genuinely produced # and accepted) must be persisted, not discarded along with it. + # validator_test_script itself is disabled by default (see + # _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT above) -- re-enabled here to + # keep the salvage mechanism (generic, not test_script-specific) covered. with patch.object(work_graph_execute, "write_workflow_script"), \ - patch.object(work_graph_execute, "execute_work_graph") as mock_execute: + patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ + patch.object(e2e_pipeline, "_VALIDATOR_MANIFEST", _VALIDATOR_MANIFEST_WITH_TEST_SCRIPT): mock_execute.side_effect = [ {"content": "# Validator's own Benchmark"}, {"content": "N/A: no dataset needed"}, diff --git a/src/test_schema.py b/src/test_schema.py index 7143b3b..f6bf877 100644 --- a/src/test_schema.py +++ b/src/test_schema.py @@ -2,6 +2,8 @@ Done criterion (unique / foreign key / enum) are actually enforced. U-034 (PH5, 2026-08-02) added `deployment`/`deploy_health_signal` (17 tables total) and widened `bug_record.root_cause_category` to accept `core_engine`. +2026-08-08 added `work_graph_loop_limit_config` (18 tables total, Work +Graph self-planning + loop-back). Run: python3 -m unittest src.test_schema -v """ @@ -21,13 +23,13 @@ class SchemaCreationTest(unittest.TestCase): def tearDown(self): self.conn.close() - def test_all_17_tables_created(self): + def test_all_18_tables_created(self): rows = self.conn.execute( "SELECT name FROM sqlite_master WHERE type = 'table' AND name NOT LIKE 'sqlite_%'" ).fetchall() actual = {row[0] for row in rows} self.assertEqual(actual, set(schema.TABLE_NAMES)) - self.assertEqual(len(schema.TABLE_NAMES), 17) + self.assertEqual(len(schema.TABLE_NAMES), 18) def test_schema_is_idempotent_to_reopen_on_disk(self): # not required by the Done criterion, but a schema that only works once @@ -737,5 +739,115 @@ class MigrateRelaxOrgNodeConstraintsTest(unittest.TestCase): fresh.close() +class MigrateAddWorkGraphLoopLimitConfigTest(unittest.TestCase): + """2026-08-08: real upgrade path for an already-populated database that + predates work_graph_loop_limit_config (plain CREATE TABLE IF NOT + EXISTS, no CHECK constraint to rebuild around).""" + + def setUp(self): + self.conn = schema.connect(":memory:") + old_sql = schema.SCHEMA_SQL.replace( + "-- 1.11.1 work_graph_loop_limit_config (Work Graph self-planning + loop-back,\n" + "-- docs/todo/work-graph-self-planning-and-loop-back-design.md §6)\n" + "CREATE TABLE work_graph_loop_limit_config (\n" + " config_id TEXT PRIMARY KEY,\n" + " max_loop_attempts INTEGER NOT NULL,\n" + " updated_by TEXT,\n" + " updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP\n" + ");\n\n", + "", + ) + self.assertNotIn("work_graph_loop_limit_config", old_sql) # sanity: substitution matched + self.conn.executescript(old_sql) + self.conn.commit() + + def tearDown(self): + self.conn.close() + + def test_migration_creates_the_table(self): + schema.migrate_add_work_graph_loop_limit_config(self.conn) + self.conn.execute( + "INSERT INTO work_graph_loop_limit_config (config_id, max_loop_attempts, updated_by) " + "VALUES ('default', 3, 'test')" + ) + self.conn.commit() + row = self.conn.execute( + "SELECT max_loop_attempts FROM work_graph_loop_limit_config WHERE config_id = 'default'" + ).fetchone() + self.assertEqual(row[0], 3) + + def test_idempotent_on_a_database_already_migrated(self): + schema.migrate_add_work_graph_loop_limit_config(self.conn) + schema.migrate_add_work_graph_loop_limit_config(self.conn) # must not raise + + def test_noop_on_a_freshly_created_database(self): + fresh = schema.connect(":memory:") + try: + schema.create_schema(fresh) + schema.migrate_add_work_graph_loop_limit_config(fresh) # must not raise + finally: + fresh.close() + + +class MigrateAddLoopBackEventTypeTest(unittest.TestCase): + """2026-08-08: real upgrade path for an already-populated database whose + execution_event table still has the pre-expansion CHECK constraint.""" + + def setUp(self): + self.conn = schema.connect(":memory:") + old_sql = schema.SCHEMA_SQL.replace( + "'log_appended', 'quota_wait_started', 'loop_back_triggered'", + "'log_appended', 'quota_wait_started'", + ) + self.assertNotIn("loop_back_triggered", old_sql) # sanity: substitution matched + self.conn.executescript(old_sql) + self.conn.commit() + self.conn.execute( + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" + ) + self.conn.execute( + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " + "VALUES ('evt-1', 'proj-1', 'log_appended', '{}')" + ) + self.conn.commit() + + def tearDown(self): + self.conn.close() + + def test_old_check_constraint_rejects_new_type_before_migration(self): + with self.assertRaises(sqlite3.IntegrityError): + self.conn.execute( + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " + "VALUES ('evt-2', 'proj-1', 'loop_back_triggered', '{}')" + ) + + def test_migration_preserves_existing_rows_and_accepts_new_type(self): + schema.migrate_add_loop_back_event_type(self.conn) + + row = self.conn.execute("SELECT event_type FROM execution_event WHERE event_id = 'evt-1'").fetchone() + self.assertEqual(row[0], "log_appended") + + self.conn.execute( + "INSERT INTO execution_event (event_id, project_id, event_type, payload) " + "VALUES ('evt-2', 'proj-1', 'loop_back_triggered', '{}')" + ) + self.conn.commit() + count = self.conn.execute("SELECT COUNT(*) FROM execution_event").fetchone()[0] + self.assertEqual(count, 2) + + def test_idempotent_on_a_database_already_migrated(self): + schema.migrate_add_loop_back_event_type(self.conn) + schema.migrate_add_loop_back_event_type(self.conn) # must not raise + + def test_noop_on_a_freshly_created_already_expanded_database(self): + fresh = schema.connect(":memory:") + try: + schema.create_schema(fresh) + schema.migrate_add_loop_back_event_type(fresh) # must not raise + finally: + fresh.close() + + if __name__ == "__main__": unittest.main() diff --git a/src/test_settings_config.py b/src/test_settings_config.py index 4e9036c..c69c6ef 100644 --- a/src/test_settings_config.py +++ b/src/test_settings_config.py @@ -49,6 +49,43 @@ class ConcurrencyConfigTest(SettingsConfigTestBase): settings_config.upsert_concurrency_config(self.conn, 50, "admin") +class LoopLimitConfigTest(SettingsConfigTestBase): + """2026-08-08: Work Graph self-planning + loop-back (docs/todo/ + work-graph-self-planning-and-loop-back-design.md §6). No implicit + 'default' config_id -- callers always pass one explicitly (they look up + "{org_node}:{node_type}", "{org_node}:*", "default" in turn).""" + + def test_get_returns_none_when_unconfigured(self): + self.assertIsNone(settings_config.get_loop_limit_config(self.conn, "planner:benchmark")) + + def test_creates_config_on_first_use(self): + result = settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") + self.assertEqual(result["max_loop_attempts"], 5) + self.assertEqual( + settings_config.get_loop_limit_config(self.conn, "planner:benchmark")["max_loop_attempts"], 5 + ) + + def test_updates_existing_config(self): + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") + result = settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 2, "admin2") + self.assertEqual(result["max_loop_attempts"], 2) + self.assertEqual( + self.conn.execute("SELECT COUNT(*) FROM work_graph_loop_limit_config").fetchone()[0], 1 + ) + + def test_different_config_ids_are_independent(self): + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 5, "admin") + settings_config.upsert_loop_limit_config(self.conn, "planner:*", 2, "admin") + self.assertEqual( + settings_config.get_loop_limit_config(self.conn, "planner:benchmark")["max_loop_attempts"], 5 + ) + self.assertEqual(settings_config.get_loop_limit_config(self.conn, "planner:*")["max_loop_attempts"], 2) + + def test_below_one_rejected(self): + with self.assertRaises(settings_config.InvalidParamError): + settings_config.upsert_loop_limit_config(self.conn, "default", 0, "admin") + + class LogRetentionConfigTest(SettingsConfigTestBase): def test_get_returns_none_when_unconfigured(self): self.assertIsNone(settings_config.get_log_retention_config(self.conn)) diff --git a/src/test_work_graph_orchestrate.py b/src/test_work_graph_orchestrate.py index f14e3ab..07e2599 100644 --- a/src/test_work_graph_orchestrate.py +++ b/src/test_work_graph_orchestrate.py @@ -130,6 +130,78 @@ class TopologicalLayersTest(unittest.TestCase): flat = [n["node_id"] for layer in layers for n in layer] self.assertEqual([n["node_id"] for n in wgo.topological_order(manifest)], flat) + def test_on_failure_edge_does_not_create_a_spurious_cycle(self): + # 2026-08-08: an on_failure edge commonly points BACKWARD (from a + # node to one of its own upstream dependencies) -- the old + # unfiltered edge merge would have folded this in as a (reversed, + # incorrect) data dependency, creating a real cycle with the + # existing forward depends_on and making this raise ValueError. + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + {"node_id": "benchmark", "depends_on": ["sdd"]}, + ], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + layers = wgo.topological_layers(manifest) + self.assertEqual([[n["node_id"] for n in layer] for layer in layers], [["sdd"], ["benchmark"]]) + + +class TransitiveDependentsTest(unittest.TestCase): + def test_includes_self_and_all_downstream(self): + manifest = { + "nodes": [ + {"node_id": "a", "depends_on": []}, + {"node_id": "b", "depends_on": ["a"]}, + {"node_id": "c", "depends_on": ["b"]}, + {"node_id": "d", "depends_on": []}, # unrelated branch + ], + "edges": [], + } + self.assertEqual(wgo._transitive_dependents(manifest, "a"), {"a", "b", "c"}) + self.assertEqual(wgo._transitive_dependents(manifest, "c"), {"c"}) + self.assertEqual(wgo._transitive_dependents(manifest, "d"), {"d"}) + + def test_ignores_on_failure_edges(self): + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + {"node_id": "benchmark", "depends_on": ["sdd"]}, + ], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + # sdd's dependents must be {sdd, benchmark} via the real depends_on + # edge, not affected by the on_failure edge going the other way. + self.assertEqual(wgo._transitive_dependents(manifest, "sdd"), {"sdd", "benchmark"}) + + +class TransitiveAncestorsTest(unittest.TestCase): + def test_includes_self_and_all_upstream(self): + manifest = { + "nodes": [ + {"node_id": "a", "depends_on": []}, + {"node_id": "b", "depends_on": ["a"]}, + {"node_id": "c", "depends_on": ["b"]}, + ], + "edges": [], + } + self.assertEqual(wgo._transitive_ancestors(manifest, "c"), {"a", "b", "c"}) + self.assertEqual(wgo._transitive_ancestors(manifest, "a"), {"a"}) + + +class OnFailureTargetTest(unittest.TestCase): + def test_finds_the_on_failure_edge_target(self): + manifest = { + "nodes": [{"node_id": "sdd"}, {"node_id": "benchmark"}], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + self.assertEqual(wgo._on_failure_target(manifest, "benchmark"), "sdd") + + def test_missing_edge_raises(self): + manifest = {"nodes": [{"node_id": "benchmark"}], "edges": []} + with self.assertRaises(ValueError): + wgo._on_failure_target(manifest, "benchmark") + class RunGraphTestBase(unittest.TestCase): def setUp(self): @@ -297,6 +369,54 @@ class RunGraphClassifierBlockedTest(RunGraphTestBase): self.assertEqual(status, "blocked") +class RunGraphUnanticipatedExceptionMarksRunBlockedTest(RunGraphTestBase): + """2026-08-08: found via a real self-planned run -- a bare exception + from build_prompt() (a caller bug, not one of the three specifically- + handled failure paths: loop_back limit exceeded, on_fail='hard_fail', + legacy _verify_and_maybe_regenerate double-failure) used to propagate + straight out of run_graph() leaving work_graph_run stuck at its + non-terminal status forever, permanently blocking every future retry + for this (task_id, org_node) via ux_work_graph_run_active's UNIQUE + index -- the real incident this test reproduces.""" + + def test_build_prompt_keyerror_still_marks_run_blocked(self): + manifest = {"nodes": [{"node_id": "sdd", "depends_on": []}], "edges": []} + + def bad_build_prompt(node, outputs): + raise KeyError("prd_content") + + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph"): + with self.assertRaises(KeyError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + bad_build_prompt, _IN_SCHEMA, _OUT_SCHEMA, + ) + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] + self.assertEqual(status, "blocked") + + def test_already_terminal_status_is_left_alone(self): + # if some inner handler already set a more specific terminal + # status before the exception reaches run_graph()'s own catch-all, + # the catch-all's WHERE clause must be a no-op, not overwrite it. + self.conn.execute("UPDATE work_graph_run SET status = 'done' WHERE run_id = 'run-1'") + self.conn.commit() + manifest = {"nodes": [{"node_id": "sdd", "depends_on": []}], "edges": []} + + def bad_build_prompt(node, outputs): + raise RuntimeError("boom") + + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph"): + with self.assertRaises(RuntimeError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + bad_build_prompt, _IN_SCHEMA, _OUT_SCHEMA, + ) + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] + self.assertEqual(status, "done") + + class VerifyFnTest(RunGraphTestBase): """REQ-02 boundary: verify_fn intercepts a bad node output before it propagates downstream, via one regeneration attempt.""" @@ -722,5 +842,420 @@ class RunGraphRealSqliteConcurrencyTest(unittest.TestCase): conn.close() +class CoerceBoolTest(unittest.TestCase): + def test_real_bool_passes_through(self): + self.assertTrue(wgo._coerce_bool(True)) + self.assertFalse(wgo._coerce_bool(False)) + + def test_string_false_is_not_truthy(self): + # regression: a real call has returned "false" as a JSON STRING + # despite "type": "boolean" in the schema -- `if "false":` is + # truthy in Python (same class of bug e2e_pipeline._coerce_bool + # was written for). + self.assertFalse(wgo._coerce_bool("false")) + + def test_string_true_is_treated_as_true(self): + self.assertTrue(wgo._coerce_bool("true")) + + +class ResolveNodeVerifyFnTest(unittest.TestCase): + """2026-08-08: node-declared `judgment` takes over verify_fn + resolution; a node with no `judgment` falls through to whatever the + caller passed as `verify_fn` (FR-A8 -- every pre-existing graph_manifest + has no `judgment` field at all).""" + + def test_no_judgment_falls_through_to_external_verify_fn(self): + external = lambda node, output: (True, None) + resolved = wgo._resolve_node_verify_fn({"node_id": "x"}, external, "planner", "/repo", None) + self.assertIs(resolved, external) + + def test_judgment_kind_none_falls_through_to_external_verify_fn(self): + external = lambda node, output: (True, None) + node = {"node_id": "x", "judgment": {"kind": "none"}} + resolved = wgo._resolve_node_verify_fn(node, external, "planner", "/repo", None) + self.assertIs(resolved, external) + + def test_no_judgment_and_no_external_verify_fn_returns_none(self): + resolved = wgo._resolve_node_verify_fn({"node_id": "x"}, None, "planner", "/repo", None) + self.assertIsNone(resolved) + + def test_mechanical_judgment_dispatches_to_registered_check(self): + node = {"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "my_check"}} + registry = {"my_check": lambda content: (content == "good", None if content == "good" else "bad content")} + resolved = wgo._resolve_node_verify_fn(node, None, "planner", "/repo", registry) + ok, feedback = resolved(node, {"content": "good"}) + self.assertTrue(ok) + ok, feedback = resolved(node, {"content": "bad"}) + self.assertFalse(ok) + self.assertEqual(feedback, ["bad content"]) + + def test_mechanical_judgment_unknown_check_ref_raises(self): + node = {"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}} + with self.assertRaises(ValueError): + wgo._resolve_node_verify_fn(node, None, "planner", "/repo", {"real_check": lambda c: (True, None)}) + + def test_unknown_judgment_kind_raises(self): + node = {"node_id": "x", "judgment": {"kind": "astrology"}} + with self.assertRaises(ValueError): + wgo._resolve_node_verify_fn(node, None, "planner", "/repo", None) + + +class ValidateMechanicalCheckRefsTest(unittest.TestCase): + """Fail before running any node, not when the offending node is reached.""" + + def test_unregistered_check_ref_raises(self): + manifest = { + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}}], + "edges": [], + } + with self.assertRaises(ValueError): + wgo._validate_mechanical_check_refs(manifest, {"real_check": lambda c: (True, None)}) + + def test_registered_check_ref_passes(self): + manifest = { + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "real_check"}}], + "edges": [], + } + wgo._validate_mechanical_check_refs(manifest, {"real_check": lambda c: (True, None)}) # must not raise + + def test_no_judgment_nodes_pass_with_no_registry(self): + manifest = {"nodes": [{"node_id": "brd"}], "edges": []} + wgo._validate_mechanical_check_refs(manifest, None) # must not raise + + +class LlmJudgmentVerifyFnTest(unittest.TestCase): + def test_non_self_target_calls_directly_without_worktree(self): + judgment = {"judge_prompt": "Review: {content}"} + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") + with patch.object(wgo.paths, "is_self_target", return_value=False), \ + patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ + patch.object( + work_graph_execute, "execute_work_graph", + return_value={"ok": True, "issues": []}, + ) as mock_execute: + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) + self.assertTrue(ok) + self.assertEqual(issues, []) + self.assertEqual(mock_write.call_args[0][0], "/repo") + self.assertEqual(mock_execute.call_args[0][0], "/repo") + + def test_string_ok_is_coerced(self): + judgment = {"judge_prompt": "Review: {content}"} + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") + with patch.object(wgo.paths, "is_self_target", return_value=False), \ + patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object( + work_graph_execute, "execute_work_graph", + return_value={"ok": "false", "issues": ["bad"]}, + ): + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) + self.assertFalse(ok) + self.assertEqual(issues, ["bad"]) + + def test_self_target_isolates_in_its_own_worktree(self): + # same pattern as e2e_pipeline.py's PrdConsistencySelfTargetWorktreeTest + judgment = {"judge_prompt": "Review: {content}"} + verify_fn = wgo._llm_judgment_verify_fn(judgment, "planner", "/repo") + with patch.object(wgo.paths, "is_self_target", return_value=True), \ + patch.object(wgo.work_node_run, "create_worktree", return_value="/repo/.graphlemon-worktrees/x") as mock_create, \ + patch.object(wgo.work_node_run, "ensure_worktree_committed") as mock_ensure, \ + patch.object(wgo.work_node_run, "merge_worktree") as mock_merge, \ + patch.object(wgo.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={"ok": True, "issues": []}, + ) as mock_execute: + ok, issues = verify_fn({"node_id": "x"}, {"content": "some content"}) + self.assertTrue(ok) + mock_create.assert_called_once_with("/repo", unittest.mock.ANY) + self.assertEqual(mock_write.call_args[0][0], "/repo/.graphlemon-worktrees/x") + self.assertEqual(mock_execute.call_args[0][0], "/repo/.graphlemon-worktrees/x") + self.assertEqual(mock_ensure.call_count, 2) + mock_merge.assert_called_once() + mock_remove.assert_called_once() + + +class RunGraphMechanicalJudgmentTest(RunGraphTestBase): + """End-to-end: a node's own judgment field (not an externally-passed + verify_fn) drives verification through run_graph().""" + + def test_node_declared_mechanical_judgment_regenerates_on_failure(self): + manifest = { + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "compiles"}}], + "edges": [], + } + registry = {"compiles": lambda content: (content == "real code", None if content == "real code" else "not code")} + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: + mock_execute.side_effect = [{"content": "narrative text"}, {"content": "real code"}] + outputs = wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, + mechanical_check_registry=registry, + ) + self.assertEqual(outputs["script"], {"content": "real code"}) + + def test_unregistered_check_ref_fails_before_any_node_runs(self): + manifest = { + "nodes": [{"node_id": "script", "judgment": {"kind": "mechanical", "check_ref": "typo"}}], + "edges": [], + } + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: + with self.assertRaises(ValueError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, + mechanical_check_registry={"compiles": lambda c: (True, None)}, + ) + mock_execute.assert_not_called() + + +class RunGraphLoopBackTest(RunGraphTestBase): + """2026-08-08, Work Graph self-planning + loop-back design §4.""" + + def test_retry_self_regenerates_with_accumulating_history(self): + # A node with on_fail.action='retry_self' loops back to itself -- + # fails twice, succeeds the third time. Each retry's prompt must + # include the FULL history of prior rejections, not just the last one. + manifest = { + "nodes": [{ + "node_id": "benchmark", + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "retry_self", "max_attempts": 5}, + }], + "edges": [], + } + contents = ["bad v1", "bad v2", "good"] + prompts_seen = [] + + def build_prompt(node, outputs): + return "base prompt" + + def fake_execute(cwd, name, args, output_schema, timeout=600): + return {"content": contents.pop(0)} + + registry = {"compiles": lambda content: (content == "good", "not real code: " + content)} + + with patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ + patch.object(work_graph_execute, "execute_work_graph", side_effect=fake_execute): + outputs = wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + build_prompt, _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + self.assertEqual(outputs["benchmark"], {"content": "good"}) + + # 3 real executions happened (2 failures + 1 success) -- 3 distinct + # work_node_execution rows, not 1 (which would mean a stale 'done' + # row got silently reused instead of genuinely re-running). + rows = self.conn.execute( + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' ORDER BY node_id" + ).fetchall() + self.assertEqual([r[0] for r in rows], ["benchmark", "benchmark-attempt2", "benchmark-attempt3"]) + + events = self.conn.execute( + "SELECT payload FROM execution_event WHERE event_type = 'loop_back_triggered' ORDER BY created_at" + ).fetchall() + self.assertEqual(len(events), 2) + payloads = [json.loads(p) for (p,) in events] + self.assertEqual(payloads[0]["attempt_number"], 1) + self.assertEqual(payloads[1]["attempt_number"], 2) + + # the THIRD (successful) execution's prompt must carry both prior + # rejections' feedback, not just the most recent one. + third_script_prompt = json.loads( + self.conn.execute( + "SELECT input_payload FROM work_node_execution WHERE node_id = 'benchmark-attempt3'" + ).fetchone()[0] + ) + # the real prompt text lives in the rendered workflow script, not + # input_payload (that's just {"node_id": ...}) -- inspect what got + # passed to write_workflow_script for the third attempt instead. + third_call_script = mock_write.call_args_list[2].args[2] + self.assertIn("not real code: bad v1", third_call_script) + self.assertIn("not real code: bad v2", third_call_script) + + def test_loop_back_to_earlier_node_reruns_target_and_downstream(self): + # benchmark declares on_fail.action='loop_back' with an on_failure + # edge back to sdd -- sdd must be genuinely re-executed (not + # short-circuited to its stale 'done' output), and benchmark's + # second attempt must see the NEW sdd output. + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + { + "node_id": "benchmark", "depends_on": ["sdd"], + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "loop_back", "max_attempts": 3}, + }, + ], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + sdd_outputs = ["sdd v1", "sdd v2 (fixed)"] + benchmark_seen_sdd = [] + + def build_prompt(node, outputs): + if node["node_id"] == "benchmark": + benchmark_seen_sdd.append(outputs["sdd"]["content"]) + return "benchmark prompt using " + outputs["sdd"]["content"] + return "sdd prompt" + + # sdd runs before benchmark in each pass (topological order), so + # call order alone deterministically identifies which node this is: + # sdd v1, bad benchmark, sdd v2 (re-run via loop_back), good benchmark. + call_order = [] + + def side_effect(cwd, name, args, output_schema, timeout=600): + call_order.append(name) + call_index = len(call_order) + if call_index == 1: + return {"content": sdd_outputs[0]} + if call_index == 2: + return {"content": "bad benchmark"} + if call_index == 3: + return {"content": sdd_outputs[1]} + return {"content": "good benchmark"} + + registry = {"compiles": lambda content: (content == "good benchmark", "rejected: " + content)} + + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph", side_effect=side_effect): + outputs = wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + build_prompt, _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + + self.assertEqual(outputs["sdd"], {"content": sdd_outputs[1]}) + self.assertEqual(outputs["benchmark"], {"content": "good benchmark"}) + self.assertEqual(benchmark_seen_sdd, [sdd_outputs[0], sdd_outputs[1]]) + + sdd_rows = self.conn.execute( + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' AND " + "(node_id = 'sdd' OR node_id LIKE 'sdd-attempt%') ORDER BY node_id" + ).fetchall() + self.assertEqual([r[0] for r in sdd_rows], ["sdd", "sdd-attempt2"]) + + def test_exceeding_limit_raises_and_blocks_run(self): + manifest = { + "nodes": [{ + "node_id": "benchmark", + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "retry_self", "max_attempts": 2}, + }], + "edges": [], + } + registry = {"compiles": lambda content: (False, "always rejected")} + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "always bad"}): + with self.assertRaises(wgo.ConsistencyCheckFailedError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + status = self.conn.execute("SELECT status FROM work_graph_run WHERE run_id = 'run-1'").fetchone()[0] + self.assertEqual(status, "blocked") + + def test_hard_fail_raises_immediately_with_no_retry(self): + manifest = { + "nodes": [{ + "node_id": "benchmark", + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "hard_fail"}, + }], + "edges": [], + } + registry = {"compiles": lambda content: (False, "bad")} + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "x"}) as mock_execute: + with self.assertRaises(wgo.ConsistencyCheckFailedError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + self.assertEqual(mock_execute.call_count, 1) # no retry at all + + def test_resume_after_restart_continues_from_the_real_attempt_count(self): + # Simulates a PRIOR process that ran attempt 1 (rejected), recorded + # the loop_back event, then died before attempt 2 ever began -- + # seeded directly via SQL rather than literally raising mid-flight + # inside execute_work_graph, which would instead exercise a + # separate, pre-existing gap in work_node_run.py's own resume + # logic (it unconditionally INSERTs a new execution_log_file row + # every call, with no existing-row check, so resuming a node_exec_id + # that already has one hits a real UNIQUE constraint violation -- + # a genuine bug, but in a different module and out of scope here). + manifest = { + "nodes": [{ + "node_id": "benchmark", + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "retry_self", "max_attempts": 5}, + }], + "edges": [], + } + registry = {"compiles": lambda content: (content == "good", "bad: " + content)} + + wgo.ensure_placeholder_skill(self.conn, "planner-doc-writing", "planner") + self.conn.execute( + "INSERT INTO work_node_execution (node_exec_id, run_id, node_id, subagent_type, skill_ref, " + "input_schema_ref, output_schema_ref, input_payload, output_payload, status) " + "VALUES ('exec-1', 'run-1', 'benchmark', 'planner', 'planner-doc-writing', " + "'work_graph_node_input.schema.json', 'work_graph_node_output.schema.json', " + "'{}', '{\"content\": \"bad v1\"}', 'done')" + ) + self.conn.execute( + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " + "VALUES ('evt-1', 'proj-1', 'task-1', 'run-1', 'loop_back_triggered', ?)", + (json.dumps({ + "source_node_id": "benchmark", "target_node_id": "benchmark", + "attempt_number": 1, "feedback": ["bad: bad v1"], + }),), + ) + self.conn.commit() + + # "restart": a fresh run_graph() call for the same run_id, this + # time succeeding -- must resume as attempt 2 (reading the true + # count from execution_event), not restart from attempt 1 or lose + # the earlier rejection's history. + with patch.object(work_graph_execute, "write_workflow_script") as mock_write, \ + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "good"}): + outputs = wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + self.assertEqual(outputs["benchmark"], {"content": "good"}) + rows = self.conn.execute( + "SELECT node_id FROM work_node_execution WHERE run_id = 'run-1' ORDER BY node_id" + ).fetchall() + self.assertEqual([r[0] for r in rows], ["benchmark", "benchmark-attempt2"]) + self.assertIn("bad: bad v1", mock_write.call_args[0][2]) + + def test_org_node_type_scoped_config_limit_used_when_no_node_level_override(self): + from src import settings_config + settings_config.upsert_loop_limit_config(self.conn, "planner:benchmark", 1, "test") + manifest = { + "nodes": [{ + "node_id": "benchmark", "artifact_type": "benchmark", + "judgment": {"kind": "mechanical", "check_ref": "compiles"}, + "on_fail": {"action": "retry_self"}, # no max_attempts -- falls back to config + }], + "edges": [], + } + registry = {"compiles": lambda content: (False, "bad")} + with patch.object(work_graph_execute, "write_workflow_script"), \ + patch.object(work_graph_execute, "execute_work_graph", return_value={"content": "x"}) as mock_execute: + with self.assertRaises(wgo.ConsistencyCheckFailedError): + wgo.run_graph( + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, mechanical_check_registry=registry, + ) + # max_attempts=1 means "1 retry allowed" (matches the legacy + # exactly-one-retry default, design doc §2's on_fail comment) -- + # attempt_number=1 on the first failure is not > 1, so one retry + # happens; attempt_number=2 on THAT failure is > 1 and exceeds. + # Two real executions total, not one. + self.assertEqual(mock_execute.call_count, 2) + + if __name__ == "__main__": unittest.main() diff --git a/src/test_work_graph_plan.py b/src/test_work_graph_plan.py index 39c7f1b..1ddd862 100644 --- a/src/test_work_graph_plan.py +++ b/src/test_work_graph_plan.py @@ -133,14 +133,15 @@ class NormalizeGraphManifestTest(unittest.TestCase): result = work_graph_plan._normalize_graph_manifest(manifest) self.assertEqual(result["nodes"][0]["depends_on"], []) - def test_canonical_doc_chain_enforced_even_with_zero_dependency_info(self): - # 2026-08-03 regression: a real call omitted depends_on AND edges - # entirely -- worse than test_depends_on_derived_from_edges_when_absent, - # which at least had edges to derive from. With nothing to - # normalize, topological_order() scheduled 'benchmark' before 'sdd' - # (alphabetically first among zero-in-degree nodes), and - # e2e_pipeline's build_prompt() crashed formatting a template that - # needed sdd's not-yet-produced output. + def test_zero_dependency_info_leaves_nodes_independent(self): + # 2026-08-08 (FR-B4 "全部自由规划"): the canonical-doc-chain forced- + # order enforcement that used to backfill this exact case (2026-08-03 + # incident) is deliberately removed, not extended -- Planner's node + # set is no longer a fixed architectural constant. A real response + # with zero dependency info now genuinely produces zero depends_on, + # same as any other node set; robustness rests entirely on the + # edge-derived backfill (test_depends_on_derived_from_edges_when_absent) + # plus _validate_loop_back_targets, not a hardcoded chain to guess from. manifest = { "nodes": [ {"node_id": "brd", "artifact_type": "brd"}, @@ -148,59 +149,142 @@ class NormalizeGraphManifestTest(unittest.TestCase): {"node_id": "sdd", "artifact_type": "sdd"}, {"node_id": "benchmark", "artifact_type": "benchmark"}, {"node_id": "test_dataset", "artifact_type": "test_dataset"}, - {"node_id": "test_script", "artifact_type": "test_script"}, ], "edges": [], } result = work_graph_plan._normalize_graph_manifest(manifest) - by_type = {n["artifact_type"]: n for n in result["nodes"]} - self.assertEqual(by_type["brd"]["depends_on"], []) - self.assertEqual(by_type["prd"]["depends_on"], ["brd"]) - self.assertEqual(by_type["sdd"]["depends_on"], ["prd"]) - self.assertEqual(by_type["benchmark"]["depends_on"], ["sdd"]) - self.assertEqual(by_type["test_dataset"]["depends_on"], ["benchmark"]) - # test_script needs both -- its own prompt references - # {benchmark_content} directly, not just what survives into - # test_dataset_content (see work_graph_plan._normalize_graph_manifest). - self.assertEqual(by_type["test_script"]["depends_on"], ["test_dataset", "benchmark"]) - - def test_canonical_chain_enforcement_uses_real_node_ids_not_artifact_types(self): - # node_id and artifact_type can differ (e.g. real LLM output naming - # nodes "n1"/"n2"/... while still setting artifact_type correctly) -- - # the enforced dependency must reference the real node_id. + for node in result["nodes"]: + self.assertEqual(node["depends_on"], []) + + def test_on_failure_edge_does_not_get_folded_into_depends_on(self): + # 2026-08-08: an on_failure edge is control-flow, not a data + # dependency -- the edge-derived depends_on backfill above must + # skip it (same reasoning as work_graph_orchestrate.py's + # topological_layers()/_transitive_dependents). manifest = { "nodes": [ - {"node_id": "step-1", "artifact_type": "brd"}, - {"node_id": "step-2", "artifact_type": "prd"}, - {"node_id": "step-3", "artifact_type": "sdd"}, - {"node_id": "step-4", "artifact_type": "benchmark"}, - {"node_id": "step-5", "artifact_type": "test_dataset"}, - {"node_id": "step-6", "artifact_type": "test_script"}, + {"node_id": "sdd", "artifact_type": "sdd"}, + { + "node_id": "benchmark", "artifact_type": "benchmark", "depends_on": ["sdd"], + "on_fail": {"action": "loop_back"}, + }, ], - "edges": [], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], } result = work_graph_plan._normalize_graph_manifest(manifest) - by_type = {n["artifact_type"]: n for n in result["nodes"]} - self.assertEqual(by_type["prd"]["depends_on"], ["step-1"]) - self.assertEqual(by_type["sdd"]["depends_on"], ["step-2"]) - self.assertEqual(by_type["benchmark"]["depends_on"], ["step-3"]) - self.assertEqual(by_type["test_dataset"]["depends_on"], ["step-4"]) - self.assertEqual(by_type["test_script"]["depends_on"], ["step-5", "step-4"]) - - def test_partial_canonical_set_does_not_trigger_enforcement(self): - # only brd/prd/sdd present, no benchmark -- not a recognized - # canonical 4-node doc chain, must not guess at a shape. + by_id = {n["node_id"]: n for n in result["nodes"]} + self.assertEqual(by_id["sdd"]["depends_on"], []) + + +class TransitiveAncestorsTest(unittest.TestCase): + def test_includes_self_and_all_upstream(self): manifest = { "nodes": [ - {"node_id": "brd", "artifact_type": "brd"}, - {"node_id": "prd", "artifact_type": "prd"}, - {"node_id": "sdd", "artifact_type": "sdd"}, + {"node_id": "a", "depends_on": []}, + {"node_id": "b", "depends_on": ["a"]}, + {"node_id": "c", "depends_on": ["b"]}, ], "edges": [], } - result = work_graph_plan._normalize_graph_manifest(manifest) - for node in result["nodes"]: - self.assertEqual(node["depends_on"], []) + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "c"), {"a", "b", "c"}) + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "a"), {"a"}) + + def test_ignores_on_failure_edges(self): + manifest = { + "nodes": [{"node_id": "sdd"}, {"node_id": "benchmark"}], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + # an on_failure edge alone (no real depends_on/data edge) must NOT + # make benchmark an ancestor of sdd. + self.assertEqual(work_graph_plan._transitive_ancestors(manifest, "sdd"), {"sdd"}) + + +class ValidateLoopBackTargetsTest(unittest.TestCase): + def test_valid_loop_back_passes(self): + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + { + "node_id": "benchmark", "depends_on": ["sdd"], + "on_fail": {"action": "loop_back"}, + }, + ], + "edges": [{"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}], + } + work_graph_plan._validate_loop_back_targets(manifest) # must not raise + + def test_no_on_failure_edge_raises(self): + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + {"node_id": "benchmark", "depends_on": ["sdd"], "on_fail": {"action": "loop_back"}}, + ], + "edges": [], + } + with self.assertRaises(work_graph_plan.PlanningFailedError): + work_graph_plan._validate_loop_back_targets(manifest) + + def test_two_on_failure_edges_from_same_node_raises(self): + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + {"node_id": "prd", "depends_on": []}, + { + "node_id": "benchmark", "depends_on": ["sdd", "prd"], + "on_fail": {"action": "loop_back"}, + }, + ], + "edges": [ + {"from": "benchmark", "to": "sdd", "edge_type": "on_failure"}, + {"from": "benchmark", "to": "prd", "edge_type": "on_failure"}, + ], + } + with self.assertRaises(work_graph_plan.PlanningFailedError): + work_graph_plan._validate_loop_back_targets(manifest) + + def test_target_not_an_upstream_dependency_raises(self): + # sdd and unrelated are siblings with no dependency relationship -- + # benchmark depends on sdd but its on_failure edge points at the + # unrelated node instead, which never influenced benchmark at all. + manifest = { + "nodes": [ + {"node_id": "sdd", "depends_on": []}, + {"node_id": "unrelated", "depends_on": []}, + { + "node_id": "benchmark", "depends_on": ["sdd"], + "on_fail": {"action": "loop_back"}, + }, + ], + "edges": [{"from": "benchmark", "to": "unrelated", "edge_type": "on_failure"}], + } + with self.assertRaises(work_graph_plan.PlanningFailedError): + work_graph_plan._validate_loop_back_targets(manifest) + + def test_target_not_in_manifest_at_all_raises(self): + manifest = { + "nodes": [{"node_id": "benchmark", "depends_on": [], "on_fail": {"action": "loop_back"}}], + "edges": [{"from": "benchmark", "to": "nonexistent", "edge_type": "on_failure"}], + } + with self.assertRaises(work_graph_plan.PlanningFailedError): + work_graph_plan._validate_loop_back_targets(manifest) + + def test_nodes_without_loop_back_are_unaffected(self): + manifest = { + "nodes": [ + {"node_id": "brd", "depends_on": []}, + {"node_id": "prd", "depends_on": ["brd"]}, + ], + "edges": [], + } + work_graph_plan._validate_loop_back_targets(manifest) # must not raise + + def test_normalize_graph_manifest_calls_validation_and_raises_on_bad_loop_back(self): + manifest = { + "nodes": [{"node_id": "benchmark", "on_fail": {"action": "loop_back"}}], + "edges": [], + } + with self.assertRaises(work_graph_plan.PlanningFailedError): + work_graph_plan._normalize_graph_manifest(manifest) class HappyPathTest(PlanWorkGraphTestBase): @@ -288,15 +372,21 @@ _CODER_THREE_MODULE_MANIFEST = { class GenericPlanningPromptTest(PlanWorkGraphTestBase): - def test_planner_still_gets_fixed_doc_chain_template(self): + def test_planner_gets_the_self_planning_catalog_template(self): + # 2026-08-08 (FR-B4 "全部自由规划"): Planner no longer gets a fixed + # "produce exactly BRD/PRD/SDD/Benchmark" instruction -- it gets the + # node type catalog and decides itself which nodes this goal needs. with patch("src.work_graph_plan.subprocess.run") as run: run.return_value = _completed_process( stdout_obj={"is_error": False, "result": _fenced_result(_MANIFEST)} ) work_graph_plan.plan_work_graph(self.conn, "planner", "task-1", "goal", "/tmp/proj-1") prompt = run.call_args.args[0][run.call_args.args[0].index("-p") + 1] - self.assertIn("BRD, PRD, SDD, Benchmark", prompt) + self.assertIn("Available node types", prompt) + self.assertIn("\"artifact_type\": \"benchmark\"", prompt) + self.assertIn("on_failure", prompt) self.assertNotIn("design-dynamic-work-graph", prompt) + self.assertNotIn("{goal_text}", prompt) # the placeholder itself must have been substituted def test_coder_gets_generic_template_referencing_the_skill(self): with patch("src.work_graph_plan.subprocess.run") as run: diff --git a/src/work_graph_orchestrate.py b/src/work_graph_orchestrate.py index d51fbe3..e5c0166 100644 --- a/src/work_graph_orchestrate.py +++ b/src/work_graph_orchestrate.py @@ -70,7 +70,7 @@ import threading import time import uuid -from src import execution_log, paths, work_graph_execute, work_node_run +from src import execution_log, paths, settings_config, work_graph_execute, work_node_run def _log_event(conn, node_exec_id, event_type, **payload): @@ -111,6 +111,22 @@ class WorkGraphLayerError(Exception): super().__init__("{} node(s) failed in one layer: {}".format(len(errors), summary)) +class _LoopBackRequested(Exception): + """Internal signal (2026-08-08, Work Graph self-planning + loop-back): + a node's on_fail.action='loop_back' fired -- run_graph()'s main loop + catches this and rewinds to source_node_id's target layer, it's not a + real failure. Raised as an exception (not returned) because it can + originate inside a ThreadPoolExecutor worker thread (a multi-node + layer) -- an exception is the only clean way to carry this out through + future.result().""" + + def __init__(self, source_node_id, target_node_id, feedback): + self.source_node_id = source_node_id + self.target_node_id = target_node_id + self.feedback = feedback + super().__init__("node {} requests loop_back to {}".format(source_node_id, target_node_id)) + + class _FetchedRows: """A pre-materialized stand-in for a sqlite3.Cursor, holding rows already fetched while _LockedConn's lock was held (see _LockedConn's @@ -191,10 +207,21 @@ def topological_layers(graph_manifest): might populate one and not the other; trusting only one would silently mis-order nodes rather than raise. Raises ValueError on a cycle or a dependency referencing an unknown node_id. + + 2026-08-08 (Work Graph self-planning + loop-back): edges with + `edge_type == "on_failure"` are control-flow (a node's failure path, + see `_on_failure_target`), not data dependencies -- they're excluded + here. An `on_failure` edge commonly points from a node back to one of + its own upstream dependencies, which the OLD unfiltered merge would + have folded in as an (incorrect, reversed) data dependency, creating a + spurious cycle that would have made this function reject the graph + before any node ever ran. """ nodes = {n["node_id"]: n for n in graph_manifest["nodes"]} deps = {node_id: set(n.get("depends_on") or []) for node_id, n in nodes.items()} for edge in graph_manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue to_id, from_id = edge.get("to"), edge.get("from") if to_id in deps and from_id in nodes: deps[to_id].add(from_id) @@ -232,6 +259,73 @@ def topological_order(graph_manifest): return [node for layer in topological_layers(graph_manifest) for node in layer] +def _transitive_dependents(graph_manifest, node_id): + """node_id itself + every node that (directly or transitively) depends + on it, via `depends_on`/normal data-dependency edges (`on_failure` + edges excluded, same reasoning as topological_layers()). Used when a + loop_back fires: node_id's output is about to be regenerated, so + everything downstream of it was computed from what's now stale input + and must be re-run too (2026-08-08, Work Graph self-planning + + loop-back design §4.2).""" + dependents = {n["node_id"]: [] for n in graph_manifest["nodes"]} + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} + for edge in graph_manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue + to_id, from_id = edge.get("to"), edge.get("from") + if to_id in deps and from_id in deps: + deps[to_id].add(from_id) + for node_id_, dep_set in deps.items(): + for dep in dep_set: + dependents.setdefault(dep, []).append(node_id_) + + affected, stack = set(), [node_id] + while stack: + current = stack.pop() + if current in affected: + continue + affected.add(current) + stack.extend(dependents.get(current, [])) + return affected + + +def _transitive_ancestors(graph_manifest, node_id): + """Reverse of _transitive_dependents: node_id itself + every node it + (directly or transitively) depends on. Used to validate that a + declared on_failure edge's target is a genuine upstream influence, not + an arbitrary node (work_graph_plan.py's _validate_loop_back_targets).""" + deps = {n["node_id"]: set(n.get("depends_on") or []) for n in graph_manifest["nodes"]} + for edge in graph_manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue + to_id, from_id = edge.get("to"), edge.get("from") + if to_id in deps and from_id in deps: + deps[to_id].add(from_id) + + ancestors, stack = set(), [node_id] + while stack: + current = stack.pop() + if current in ancestors: + continue + ancestors.add(current) + stack.extend(deps.get(current, [])) + return ancestors + + +def _on_failure_target(graph_manifest, node_id): + """The node_id that `node_id`'s on_fail.action='loop_back' should + return to -- read from the single `{"from": node_id, "to": ..., + "edge_type": "on_failure"}` edge, not a separate node field (2026-08-08 + design revision: the failure path is expressed the same way the + success path already is, via `edges`). Callers should have already + validated (work_graph_plan.py's _validate_loop_back_targets) that + exactly one such edge exists before relying on this at runtime.""" + for edge in graph_manifest.get("edges", []): + if edge.get("from") == node_id and edge.get("edge_type") == "on_failure": + return edge["to"] + raise ValueError("node {} declares on_fail.action='loop_back' but has no on_failure edge".format(node_id)) + + def ensure_placeholder_skill(conn, skill_id, owner_org_node): """No Skill content has actually been migrated from the Hermes profiles yet (skill_update.py's own documented gap) -- work_node_execution. @@ -302,6 +396,221 @@ def _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, outpu return run_fn +def _coerce_bool(value): + """Same fix as e2e_pipeline.py's own _coerce_bool (2026-08-02): a real + call has returned e.g. "false" as a JSON STRING despite "type": + "boolean" in the schema, and `if "false":` is truthy in Python. Kept as + its own small copy here rather than importing e2e_pipeline (which + imports this module -- importing back would be circular) or promoting + it to a shared utils module for a 3-line helper.""" + if isinstance(value, bool): + return value + if isinstance(value, str): + return value.strip().lower() == "true" + return bool(value) + + +_JUDGMENT_SCHEMA = { + "type": "object", + "required": ["ok", "issues"], + "properties": {"ok": {"type": "boolean"}, "issues": {"type": "array", "items": {"type": "string"}}}, +} + + +def _llm_judgment_verify_fn(judgment, org_node, target_folder_path, timeout=180): + """judgment.kind == 'llm': generalizes e2e_pipeline.py's + _check_prd_consistency -- same self-target worktree isolation (a real + agent() call with memory:project writes an uncommitted change wherever + it runs; self-target runs need their own worktree so that doesn't land + on the main checkout, see that function's own docstring for the + 2026-08-03 incident this guards against), but the judge prompt comes + from judgment['judge_prompt'] (node-declared, self-contained -- same + design principle as _GENERIC_PROMPT_TEMPLATE's task_prompt) instead of + a hardcoded PRD-specific prompt, and the schema is the generic + {"ok": bool, "issues": [...]} shape instead of PRD's {"consistent": ...}. + e2e_pipeline.py's own _check_prd_consistency/_prd_consistency_verify_fn + are untouched and remain the path Validator's fixed manifest uses + (FR-A8) -- this is a parallel, more general implementation, not a + replacement. `is_self_target` is checked fresh on each call (not cached + at closure-creation time) -- matches _check_prd_consistency's own + behavior, and means a caller building this verify_fn once and reusing + it always reflects the real current state, not a stale snapshot.""" + + def verify_fn(node, output): + is_self = paths.is_self_target(target_folder_path) + node_exec_id = "judgment-" + 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: + prompt = judgment["judge_prompt"].format(content=output["content"]) + script_name = "graphlemon-judgment-" + uuid.uuid4().hex + script_content = ( + "export const meta = {{ name: {name}, description: 'graphlemon node judgment' }}\n\n" + "const result = await agent({prompt}, {{ agentType: {org_node_json}, schema: {schema} }})\n\n" + "return result\n" + ).format( + name=json.dumps(script_name), prompt=json.dumps(prompt), + org_node_json=json.dumps(org_node), schema=json.dumps(_JUDGMENT_SCHEMA), + ) + work_graph_execute.write_workflow_script(cwd, script_name, script_content) + result = work_graph_execute.execute_work_graph(cwd, script_name, {}, _JUDGMENT_SCHEMA, timeout=timeout) + if worktree_path: + work_node_run.ensure_worktree_committed(worktree_path, node_exec_id) + work_node_run.ensure_worktree_committed(target_folder_path, node_exec_id + "-main-checkout-drift") + work_node_run.merge_worktree(target_folder_path, node_exec_id) + ok = _coerce_bool(result["ok"]) + return ok, (result.get("issues") or []) + finally: + if worktree_path: + work_node_run.remove_worktree(target_folder_path, worktree_path) + + return verify_fn + + +def _resolve_node_verify_fn(node, external_verify_fn, org_node, target_folder_path, mechanical_check_registry): + """A node's own `judgment` field (if declared) takes over verify_fn + resolution for that node; a node with no `judgment` (every existing + graph_manifest today) falls through to whatever the caller passed as + `verify_fn` -- possibly None, meaning no verification at all -- exactly + like current behavior (FR-A8).""" + judgment = node.get("judgment") + if not judgment or judgment.get("kind", "none") == "none": + return external_verify_fn + if judgment["kind"] == "mechanical": + check_fn = (mechanical_check_registry or {}).get(judgment["check_ref"]) + if check_fn is None: + raise ValueError("unknown mechanical check_ref: {!r}".format(judgment["check_ref"])) + + def mechanical_verify_fn(node, output, _check_fn=check_fn): + ok, detail = _check_fn(output["content"]) + return ok, ([detail] if detail else None) + + return mechanical_verify_fn + if judgment["kind"] == "llm": + return _llm_judgment_verify_fn(judgment, org_node, target_folder_path) + raise ValueError("unknown judgment.kind: {!r}".format(judgment["kind"])) + + +def _validate_mechanical_check_refs(graph_manifest, mechanical_check_registry): + """Fail fast before running ANY node -- a typo'd check_ref caught only + when that node is finally reached would waste every token spent on the + nodes before it.""" + registry = mechanical_check_registry or {} + for node in graph_manifest["nodes"]: + judgment = node.get("judgment") or {} + if judgment.get("kind") == "mechanical" and judgment.get("check_ref") not in registry: + raise ValueError( + "node {}'s judgment.check_ref {!r} is not in mechanical_check_registry".format( + node["node_id"], judgment.get("check_ref") + ) + ) + + +_DEFAULT_LOOP_LIMIT = 3 # only reached if even the "default" config_id row is unseeded + + +def _resolve_loop_limit(conn, org_node, graph_manifest, source_node_id): + """How many times `source_node_id` (the node whose judgment failed and + declared on_fail.action in {'retry_self', 'loop_back'}) may trigger a + loop before run_graph() gives up. Lookup order (2026-08-08 design §6): + the failing node's own on_fail.max_attempts, then a settings_config.py + row keyed by "{org_node}:{node_type}", then "{org_node}:*", then + "default", then a hardcoded fallback.""" + source_node = next(n for n in graph_manifest["nodes"] if n["node_id"] == source_node_id) + node_level = (source_node.get("on_fail") or {}).get("max_attempts") + if node_level is not None: + return node_level + node_type = source_node.get("artifact_type", source_node_id) + for config_id in ("{}:{}".format(org_node, node_type), "{}:*".format(org_node), "default"): + config = settings_config.get_loop_limit_config(conn, config_id) + if config: + return config["max_loop_attempts"] + return _DEFAULT_LOOP_LIMIT + + +def _record_loop_back_event(conn, run_id, sig, attempt_number): + """Persists one 'loop_back_triggered' execution_event row -- this IS + the loop history (2026-08-08 design §7): no separate table, so a + node's full "sent back N times, here's why each time" story is + reconstructed purely by querying execution_event.""" + row = conn.execute( + "SELECT wgr.task_id, t.project_id FROM work_graph_run wgr " + "JOIN task t ON wgr.task_id = t.task_id WHERE wgr.run_id = ?", (run_id,) + ).fetchone() + task_id, project_id = row + conn.execute( + "INSERT INTO execution_event (event_id, project_id, task_id, run_id, event_type, payload) " + "VALUES (?, ?, ?, ?, 'loop_back_triggered', ?)", + (str(uuid.uuid4()), project_id, task_id, run_id, json.dumps({ + "source_node_id": sig.source_node_id, "target_node_id": sig.target_node_id, + "attempt_number": attempt_number, "feedback": sig.feedback, + })), + ) + conn.commit() + + +def _loop_back_events(conn, run_id): + rows = conn.execute( + "SELECT payload FROM execution_event WHERE run_id = ? AND event_type = 'loop_back_triggered' " + "ORDER BY created_at", (run_id,) + ).fetchall() + return [json.loads(p) for (p,) in rows] + + +def _loop_attempt_count_from_events(conn, run_id, target_node_id, source_node_id): + """Real count of prior loop_back triggers for this exact (target, + source) pair, read from execution_event -- not an in-memory counter, + so a process restart mid-loop resumes with the true count (REQ-06).""" + return sum( + 1 for e in _loop_back_events(conn, run_id) + if e["target_node_id"] == target_node_id and e["source_node_id"] == source_node_id + ) + + +def _loop_history_prompt_suffix(conn, run_id, node_id): + """When node_id is about to be (re-)executed, append the full history + of every prior time it was sent back for revision (not just the most + recent) -- empty string if node_id was never a loop_back target.""" + history = [e for e in _loop_back_events(conn, run_id) if e["target_node_id"] == node_id] + if not history: + return "" + lines = [ + "Attempt {}: rejected because: {}".format(h["attempt_number"], "; ".join(h["feedback"] or [])) + for h in history + ] + return "\n\nThis node was sent back for revision before, with this history:\n" + "\n".join(lines) + + +def _next_exec_node_id(conn, run_id, node_id, graph_manifest): + """The exec node_id to use for node_id's NEXT real execution. A node + that has never had its cached output invalidated by a loop_back (the + overwhelming common case, and every node in a graph_manifest with no + judgment/on_fail at all) always reuses the plain node_id -- preserving + run_work_node's normal breakpoint-resume short-circuit (a 'done' row + for node_id returns its persisted output instead of re-running) + completely unchanged, including across a process restart on a run + that never looped at all. + + 2026-08-08 (found via a real test failure, not just theory): counting + only "was node_id EVER the direct loop_back target" undercounts -- + every node in `_transitive_dependents(target)` gets its `outputs` + entry popped and genuinely re-executed too (run_graph()'s own + control flow), not just the target itself. A downstream node that's + never itself a `target_node_id` but IS swept up by some earlier + node's loop_back still needs its own fresh '-attemptN' row each time, + or it silently short-circuits to its own stale pre-invalidation + output on the second pass. So this counts every loop_back event + whose target's `_transitive_dependents` set includes node_id, not + just events where node_id itself is the literal target.""" + prior_invalidations = sum( + 1 for e in _loop_back_events(conn, run_id) + if node_id in _transitive_dependents(graph_manifest, e["target_node_id"]) + ) + if prior_invalidations == 0: + return node_id + return "{}-attempt{}".format(node_id, prior_invalidations + 1) + + _DEFAULT_MAX_WORKERS = 4 # see this module's own docstring section on G for rationale @@ -309,6 +618,7 @@ def run_graph( conn, run_id, org_node, skill_id, target_folder_path, graph_manifest, build_prompt, input_schema, output_schema, timeout=600, needs_worktree=False, verify_fn=None, sleep=time.sleep, now=time.time, max_workers=_DEFAULT_MAX_WORKERS, + mechanical_check_registry=None, ): """Runs every node in `graph_manifest` through `run_work_node()`, each backed by a real one-node dynamic-workflow `agent()` call using @@ -372,75 +682,225 @@ def run_graph( even after every sibling has finished -- Python threads have no clean preemptible cancel, and building one is out of scope for this pass. + `mechanical_check_registry` (2026-08-08, Work Graph self-planning + + loop-back): {check_ref: callable(content) -> (ok, detail)}, consulted + by any node declaring `judgment: {"kind": "mechanical", "check_ref": + ...}` -- see `_resolve_node_verify_fn`. A node with no `judgment` field + (every graph_manifest that predates this) ignores this entirely and + keeps using whatever `verify_fn` the caller passed, unchanged (FR-A8). + + **loop_back (2026-08-08, Work Graph self-planning + loop-back)**: a + node declaring its own `judgment` can also declare `on_fail.action`: + `hard_fail` (fail the whole run immediately, no retry), `retry_self` + (loop back to itself), or `loop_back` (jump to an earlier node named + by that node's own `edge_type: "on_failure"` edge -- see + `_on_failure_target`). `retry_self`/`loop_back` share one mechanism: + the target node (and everything transitively depending on it, since + their input is now stale) is re-executed with the full accumulated + history of every prior rejection appended to its prompt + (`_loop_history_prompt_suffix`), bounded per (target, source) pair by + `_resolve_loop_limit` -- exceeding it raises `ConsistencyCheckFailedError` + same as the legacy single-retry path. A node with NO `judgment` field + (every graph_manifest that predates this) is entirely unaffected -- + it keeps going through `_verify_and_maybe_regenerate` exactly as + before, regardless of any of this (FR-A8). + Returns {node_id: output_payload}. """ + _validate_mechanical_check_refs(graph_manifest, mechanical_check_registry) ensure_placeholder_skill(conn, skill_id, org_node) effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path) layers = topological_layers(graph_manifest) + node_layer_index = {node["node_id"]: i for i, layer in enumerate(layers) for node in layer} outputs = {} locked_conn = _LockedConn(conn) - for layer in layers: - if len(layer) == 1: - node = layer[0] - outputs[node["node_id"]] = _run_one_node( - conn, run_id, org_node, skill_id, target_folder_path, node, outputs, - build_prompt, input_schema, output_schema, timeout, - effective_needs_worktree, verify_fn, sleep, now, - ) + try: + return _run_layers( + conn, run_id, org_node, skill_id, target_folder_path, layers, node_layer_index, outputs, + graph_manifest, build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, + verify_fn, sleep, now, max_workers, mechanical_check_registry, locked_conn, + ) + except Exception: + # 2026-08-08 (found via a real self-planned run): only three specific, + # anticipated failure paths (loop_back exceeding its limit, + # on_fail.action='hard_fail', the legacy _verify_and_maybe_regenerate + # double-failure) ever mark work_graph_run terminal. Any OTHER + # exception -- e.g. a bare KeyError from a caller's build_prompt(), + # which is exactly what a real run hit -- used to propagate straight + # out of run_graph() leaving the row stuck at its non-terminal status + # forever, permanently blocking every future attempt for this + # (task_id, org_node) via ux_work_graph_run_active's UNIQUE index + # (schema.py) -- the retry itself would fail immediately on the + # INSERT, with no clean error explaining why. This is a catch-all + # safety net, not a substitute for the three specific handlers above + # (which run first and set more specific end-states along the way); + # the WHERE clause is a no-op if one of them (or a nested call) had + # already marked this run terminal. + conn.execute( + "UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ? " + "AND status IN ('planning', 'running', 'merging', 'verifying', 'reflecting')", + (run_id,), + ) + conn.commit() + raise + + +def _run_layers( + conn, run_id, org_node, skill_id, target_folder_path, layers, node_layer_index, outputs, + graph_manifest, build_prompt, input_schema, output_schema, timeout, effective_needs_worktree, + verify_fn, sleep, now, max_workers, mechanical_check_registry, locked_conn, +): + """run_graph()'s actual node-by-node/layer-by-layer execution loop -- + split out so run_graph() itself can wrap the whole thing in one + try/except (see its own comment) without deeply nesting this loop.""" + layer_index = 0 + while layer_index < len(layers): + pending = [node for node in layers[layer_index] if node["node_id"] not in outputs] + if not pending: + layer_index += 1 continue - worker_count = min(max_workers, len(layer)) - errors = [] - with concurrent.futures.ThreadPoolExecutor(max_workers=worker_count) as pool: - futures = { - pool.submit( - _run_one_node, locked_conn, run_id, org_node, skill_id, target_folder_path, - node, outputs, build_prompt, input_schema, output_schema, timeout, - effective_needs_worktree, verify_fn, sleep, now, - ): node - for node in layer - } - done, _ = concurrent.futures.wait(futures, return_when=concurrent.futures.ALL_COMPLETED) - for future in done: - node = futures[future] - try: - outputs[node["node_id"]] = future.result() - except Exception as e: - errors.append((node["node_id"], e)) - - if errors: - raise WorkGraphLayerError(errors) + loop_back_signal = None + if len(pending) == 1: + node = pending[0] + try: + outputs[node["node_id"]] = _run_one_node( + conn, run_id, org_node, skill_id, target_folder_path, node, outputs, graph_manifest, + build_prompt, input_schema, output_schema, timeout, + effective_needs_worktree, verify_fn, sleep, now, mechanical_check_registry, + ) + except _LoopBackRequested as sig: + loop_back_signal = sig + else: + worker_count = min(max_workers, len(pending)) + errors = [] + loop_back_signals = [] + with concurrent.futures.ThreadPoolExecutor(max_workers=worker_count) as pool: + futures = { + pool.submit( + _run_one_node, locked_conn, run_id, org_node, skill_id, target_folder_path, + node, outputs, graph_manifest, build_prompt, input_schema, output_schema, timeout, + effective_needs_worktree, verify_fn, sleep, now, mechanical_check_registry, + ): node + for node in pending + } + done, _ = concurrent.futures.wait(futures, return_when=concurrent.futures.ALL_COMPLETED) + for future in done: + node = futures[future] + try: + outputs[node["node_id"]] = future.result() + except _LoopBackRequested as sig: + loop_back_signals.append(sig) + except Exception as e: + errors.append((node["node_id"], e)) + + if loop_back_signals: + # A layer can have at most one *winning* rewind per pass -- + # pick deterministically (lowest source_node_id). Any plain + # `errors` from OTHER siblings in this same pass are + # dropped, not raised: they'll surface again naturally on + # the next pass if still genuinely wrong after the rewind + # (same "accept a known, documented limitation rather than + # build a bigger coordinator" philosophy as the quota-wait + # limitation already documented above for G). + loop_back_signal = sorted(loop_back_signals, key=lambda s: s.source_node_id)[0] + elif errors: + raise WorkGraphLayerError(errors) + + if loop_back_signal is not None: + sig = loop_back_signal + attempt_number = _loop_attempt_count_from_events( + conn, run_id, sig.target_node_id, sig.source_node_id + ) + 1 + limit = _resolve_loop_limit(conn, org_node, graph_manifest, sig.source_node_id) + if attempt_number > limit: + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) + conn.commit() + raise ConsistencyCheckFailedError( + "loop_back from {} to {} exceeded max_attempts={}".format( + sig.source_node_id, sig.target_node_id, limit + ), + node_id=sig.source_node_id, + ) + _record_loop_back_event(conn, run_id, sig, attempt_number) + for affected_id in _transitive_dependents(graph_manifest, sig.target_node_id): + outputs.pop(affected_id, None) + layer_index = node_layer_index[sig.target_node_id] + continue + + layer_index += 1 return outputs def _run_one_node( - conn, run_id, org_node, skill_id, target_folder_path, node, outputs, + conn, run_id, org_node, skill_id, target_folder_path, node, outputs, graph_manifest, build_prompt, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, + mechanical_check_registry=None, ): """One node's full lifecycle (row creation, prompt, run, optional - verify+regenerate) -- shared by both the synchronous single-node-layer - path and the ThreadPoolExecutor multi-node-layer path in run_graph(), - so there's exactly one implementation of "run a node" to maintain.""" + verify+regenerate/loop_back) -- shared by both the synchronous + single-node-layer path and the ThreadPoolExecutor multi-node-layer + path in run_graph(), so there's exactly one implementation of "run a + node" to maintain. + + 2026-08-08: uses `_next_exec_node_id` rather than always the plain + node_id -- see that function's own docstring for why (a loop_back + target must never collide with, and short-circuit against, its own + stale pre-loop_back 'done' row; every other node keeps resuming + exactly like before).""" node_id = node["node_id"] + exec_node_id_str = _next_exec_node_id(conn, run_id, node_id, graph_manifest) + node_exec_id = _ensure_node_row( - conn, run_id, node_id, org_node, skill_id, + conn, run_id, exec_node_id_str, org_node, skill_id, "work_graph_node_input.schema.json", "work_graph_node_output.schema.json", {"node_id": node_id}, ) - prompt = build_prompt(node, outputs) + prompt = build_prompt(node, outputs) + _loop_history_prompt_suffix(conn, run_id, node_id) run_fn = _make_run_fn(conn, target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) output = work_node_run.run_work_node( conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, needs_worktree=needs_worktree, repo_path=target_folder_path, sleep=sleep, now=now, ) - if verify_fn is not None: - output = _verify_and_maybe_regenerate( + + judgment = node.get("judgment") + has_own_judgment = bool(judgment) and judgment.get("kind", "none") != "none" + resolved_verify_fn = _resolve_node_verify_fn( + node, verify_fn, org_node, target_folder_path, mechanical_check_registry + ) + if resolved_verify_fn is None: + return output + + if not has_own_judgment: + # Legacy path (FR-A8): this node has no judgment of its own -- + # whatever external verify_fn the caller passed behaves exactly as + # before (_verify_and_maybe_regenerate does its OWN verify_fn + # call internally -- checking here too would call it twice for + # every node, once redundantly), regardless of any on_fail field + # (legacy nodes don't have one). + return _verify_and_maybe_regenerate( conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, - output, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, + output, input_schema, output_schema, timeout, needs_worktree, resolved_verify_fn, sleep, now, + ) + + ok, feedback = resolved_verify_fn(node, output) + if ok: + return output + + action = (node.get("on_fail") or {}).get("action", "retry_self") + if action == "hard_fail": + conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) + conn.commit() + raise ConsistencyCheckFailedError( + "node {} failed judgment with on_fail.action='hard_fail': {}".format( + node_id, "; ".join(feedback or []) + ), + node_id=node_id, ) - return output + target = node_id if action == "retry_self" else _on_failure_target(graph_manifest, node_id) + raise _LoopBackRequested(source_node_id=node_id, target_node_id=target, feedback=feedback) def _verify_and_maybe_regenerate( diff --git a/src/work_graph_plan.py b/src/work_graph_plan.py index e4fa23e..409e2e4 100644 --- a/src/work_graph_plan.py +++ b/src/work_graph_plan.py @@ -46,6 +46,24 @@ GRAPH_MANIFEST_SCHEMA = { "enum": ["brd", "prd", "sdd", "benchmark", "test_dataset", "test_script"], }, "depends_on": {"type": "array", "items": {"type": "string"}}, + # 2026-08-08 (Work Graph self-planning + loop-back) -- both optional, + # documentation only (this schema isn't jsonschema-enforced, see + # module docstring's --json-schema/--agent note). + "judgment": { + "type": "object", + "properties": { + "kind": {"type": "string", "enum": ["none", "mechanical", "llm"]}, + "check_ref": {"type": "string"}, + "judge_prompt": {"type": "string"}, + }, + }, + "on_fail": { + "type": "object", + "properties": { + "action": {"type": "string", "enum": ["retry_self", "loop_back", "hard_fail"]}, + "max_attempts": {"type": "integer"}, + }, + }, }, }, }, @@ -57,24 +75,87 @@ GRAPH_MANIFEST_SCHEMA = { "properties": { "from": {"type": "string"}, "to": {"type": "string"}, - "edge_type": {"type": "string", "enum": ["zero_token", "needs_judgment"]}, + # "on_failure" (2026-08-08): a node's failure path, expressed the + # same way its success path already is -- see _on_failure_target + # in work_graph_orchestrate.py. + "edge_type": {"type": "string", "enum": ["zero_token", "needs_judgment", "on_failure"]}, }, }, }, }, } +# 2026-08-08 (Work Graph self-planning + loop-back, docs/todo/work-graph- +# self-planning-and-loop-back-requirements.md, FR-B4 "全部自由规划"): +# Planner's doc-writing chain is no longer a fixed architectural constant +# (formally reverses docs/done/dynamic-work-graph-design-skill-design.md +# §3's decision) -- the LLM decides which node types this goal needs, in +# what order, and how each one's judgment/on_fail should work. This +# catalog is reference DATA the prompt hands the LLM, not a requirement +# that every entry appears -- FR-B2. test_script (disabled 2026-08-07, +# docs/todo/validator-test-script-narrative-output-issue.md) is back in +# this catalog per that decision's §8 item 7: Part B doesn't wait for the +# narrative-output structural fix, that fix is tracked separately +# alongside a future Coder-side test_script module. +_PRD_JUDGE_PROMPT = ( + "Review the following PRD for internally CONTRADICTORY scenario assumptions -- e.g. two " + "requirements that cannot both hold at once, or the same scenario described two incompatible " + "ways. Do NOT flag mere incompleteness, ambiguity, or style issues, only genuine " + "self-contradiction. Reply via the required schema: ok=false only if you found a real " + "contradiction, and list each one in issues.\n\nPRD:\n\n{content}" +) + +_PLANNER_NODE_CATALOG = [ + {"artifact_type": "brd", "description": "Business Requirements Document -- why this goal matters, who it's for."}, + { + "artifact_type": "prd", "description": "Product Requirements Document -- concrete scenarios/behavior.", + "suggested_judgment": {"kind": "llm", "judge_prompt": _PRD_JUDGE_PROMPT}, + }, + {"artifact_type": "sdd", "description": "Solution Design Document -- how it will be implemented."}, + {"artifact_type": "benchmark", "description": "Concrete acceptance scenarios with observable pass criteria."}, + { + "artifact_type": "test_dataset", + "description": "Concrete test data (inputs + expected outputs) if this goal has a natural one -- " + "many goals (UI wiring, config changes) don't, that's fine, omit this node then.", + }, + { + "artifact_type": "test_script", + "description": "A real, runnable script that mechanically checks the Benchmark's scenarios -- " + "only include this if a genuine mechanical check is possible for this goal.", + "suggested_judgment": {"kind": "mechanical", "check_ref": "check_script_compiles"}, + }, +] + +# NOTE: no `str.format()` involved in building or filling this template +# (unlike _GENERIC_PROMPT_TEMPLATE) -- the embedded catalog JSON and the +# instructional JSON examples below (e.g. {"kind": "none"}) are full of +# their own literal braces, which .format() would try to parse as format +# fields and crash on. plan_work_graph() fills in goal_text with a plain +# `.replace("{goal_text}", goal_text)` instead, which treats every other +# brace in this string as inert literal text -- no escaping needed at all. _PROMPT_TEMPLATE = ( - "Plan (do not execute) the Work Graph needed to produce BRD, PRD, SDD, Benchmark, " - "Test Dataset, and Test Script artifacts for this goal: {goal_text}\n" - "Output a graph_manifest with one node per artifact_type (brd, prd, sdd, benchmark, " - "test_dataset, test_script) and edges expressing their dependency chain (sdd depends " - "on prd depends on brd, benchmark depends on sdd, test_dataset depends on benchmark, " - "test_script depends on test_dataset). Every edge needs edge_type: 'zero_token' for " - "pure aggregation/reduce steps, 'needs_judgment' for anything requiring the agent to " - "actually reason about content.\n" + "Plan (do not execute) the Work Graph for this goal: {goal_text}\n" + "Available node types (use any subset, in any order that makes sense -- this goal may not " + "need all of them, and 'suggested_judgment' entries are starting points you can reuse or " + "override, not requirements):\n" + json.dumps(_PLANNER_NODE_CATALOG) + "\n" + "For each node you include, decide: node_id, artifact_type, depends_on (which earlier nodes' " + "output it genuinely needs), a judgment config (how to tell if this node's output is " + "acceptable -- {\"kind\": \"none\"} if nothing needs checking, {\"kind\": \"mechanical\", " + "\"check_ref\": ...} for a named mechanical check, or {\"kind\": \"llm\", \"judge_prompt\": " + "\"...{content}...\"} for your own judgment prompt with a {content} placeholder), and an " + "on_fail config (what happens if judgment fails: {\"action\": \"retry_self\"}, " + "{\"action\": \"loop_back\"}, or {\"action\": \"hard_fail\"}, each optionally with " + "\"max_attempts\"). If a node's on_fail.action is 'loop_back', you MUST also add a " + "corresponding edge in the top-level 'edges' array with edge_type 'on_failure', from that " + "node to the exact earlier node_id it should return to -- the target must be a real upstream " + "dependency of this node (something that genuinely produced input this node used), not an " + "arbitrary node. This mirrors how the success path already works: every node's 'yes, this " + "succeeded, what's next' is expressed as a normal dependency edge, and 'no, this failed, go " + "back to X' is expressed the same way, just with edge_type 'on_failure' instead. Every other " + "edge needs edge_type: 'zero_token' for pure aggregation/reduce steps, 'needs_judgment' for " + "anything requiring the agent to actually reason about content.\n" "End your reply with the graph_manifest as a single fenced ```json code block " - "shaped exactly like {{\"nodes\": [...], \"edges\": [...]}} -- no extra wrapper key." + "shaped exactly like {\"nodes\": [...], \"edges\": [...]} -- no extra wrapper key." ) # E (2026-08-06): the generic, cross-Org-Agent path -- used for every @@ -134,7 +215,72 @@ def _extract_graph_manifest(text): return None -_CANONICAL_DOC_CHAIN = ("brd", "prd", "sdd", "benchmark", "test_dataset", "test_script") +def _transitive_ancestors(graph_manifest, node_id): + """node_id itself + every node it (directly or transitively) depends + on, via `depends_on`/normal data-dependency edges (`on_failure` edges + excluded -- they're control-flow, not data dependency, same reasoning + as work_graph_orchestrate.py's own `_transitive_dependents`/ + `topological_layers`). Deliberately reimplemented here rather than + imported from work_graph_orchestrate.py -- the two modules don't + depend on each other today (work_graph_orchestrate.py is a lower-level + execution engine, work_graph_plan.py is planning-only) and this is a + small enough function that duplicating it doesn't justify introducing + that coupling.""" + deps = {n.get("node_id"): set(n.get("depends_on") or []) for n in graph_manifest.get("nodes", [])} + for edge in graph_manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue + from_id, to_id = edge.get("from"), edge.get("to") + if to_id in deps and from_id in deps: + deps[to_id].add(from_id) + + ancestors, stack = set(), [node_id] + while stack: + current = stack.pop() + if current in ancestors: + continue + ancestors.add(current) + stack.extend(deps.get(current, [])) + return ancestors + + +def _validate_loop_back_targets(manifest): + """2026-08-08 (Work Graph self-planning + loop-back): every node + declaring `on_fail.action == 'loop_back'` must have exactly one + `edge_type: 'on_failure'` edge originating from it (see + work_graph_orchestrate.py's `_on_failure_target`), and that edge's + target must be a genuine upstream dependency -- looping back to a node + that never influenced this one has no coherent meaning. Raises + PlanningFailedError (not a generic ValueError) so this surfaces the + same way every other planning failure does, before any node ever runs + -- catching a malformed loop_back here is far cheaper than discovering + it mid-execution.""" + node_ids = {n.get("node_id") for n in manifest.get("nodes", [])} + for node in manifest.get("nodes", []): + if (node.get("on_fail") or {}).get("action") != "loop_back": + continue + node_id = node.get("node_id") + on_failure_edges = [ + e for e in manifest.get("edges", []) + if e.get("from") == node_id and e.get("edge_type") == "on_failure" + ] + if len(on_failure_edges) != 1: + raise PlanningFailedError( + "node {} declares on_fail.action='loop_back' but has {} on_failure edge(s), " + "expected exactly 1".format(node_id, len(on_failure_edges)) + ) + target = on_failure_edges[0].get("to") + if target not in node_ids: + raise PlanningFailedError( + "node {}'s on_failure edge targets {!r}, which does not exist in this graph_manifest".format( + node_id, target + ) + ) + if target not in _transitive_ancestors(manifest, node_id): + raise PlanningFailedError( + "node {}'s on_failure edge targets {!r}, which is not an upstream dependency -- " + "loop_back must target a node that genuinely influenced this one".format(node_id, target) + ) def _normalize_graph_manifest(manifest): @@ -150,23 +296,22 @@ def _normalize_graph_manifest(manifest): depends_on or edges" fallback logic repeated independently. Mutates and returns `manifest`. - **Canonical doc-chain enforcement (2026-08-03, real self-hosting run)**: - a real call was observed to omit the brd->prd->sdd->benchmark - dependency entirely -- no `depends_on`, no `edges` connecting them at - all, not just the wrong key names the edge-derived backfill above can - already recover from. With no dependency info to normalize from, - `topological_order()` scheduled `benchmark` before `sdd` even ran - (alphabetically first among nodes with an empty in-degree), and - `e2e_pipeline.build_prompt()` crashed with `KeyError: 'sdd_content'` - trying to format a template that needed sdd's not-yet-produced output. - `_PROMPT_TEMPLATE` always asks for exactly this fixed 4-node linear - chain -- it's a constant architectural fact about what the Planner's - doc-writing graph looks like, not something that genuinely varies call - to call, so don't leave it to chance: whenever all four canonical - artifact_types are present, deterministically enforce the known linear - order regardless of what edges/depends_on the LLM did or didn't - provide. Same "机制用代码" principle already applied elsewhere - (`work_node_run.ensure_worktree_committed`, etc.). + 2026-08-08 (Work Graph self-planning + loop-back, FR-B4 "全部自由规划"): + the canonical-doc-chain forced-order enforcement that used to live here + (2026-08-03 incident: a real call omitted brd->prd->sdd->benchmark's + dependency entirely, scheduling nodes out of order) is REMOVED, not + extended -- Planner's node set/order is no longer a fixed architectural + constant (formally reverses docs/done/dynamic-work-graph-design-skill- + design.md §3). Robustness against that same failure mode now rests + entirely on the edge-derived depends_on backfill below (unchanged) plus + `_validate_loop_back_targets` (new, below) -- there is no longer a + hardcoded canonical chain to fall back on if an LLM response omits + dependency info, so a real reproduction of the 2026-08-03 incident + shape is a genuine, currently-unmitigated risk this design accepted + (see docs/todo/work-graph-self-planning-and-loop-back-requirements.md + §7's risk list) -- FR-B3's own acceptance criterion only covers the + "wrong key names" failure mode this backfill already handles, not a + complete absence of dependency info with no canonical chain to guess from. """ nodes = manifest.get("nodes", []) for node in nodes: @@ -176,33 +321,15 @@ def _normalize_graph_manifest(manifest): node_ids = {n.get("node_id") for n in nodes} for edge in manifest.get("edges", []): + if edge.get("edge_type") == "on_failure": + continue # control-flow, not a data dependency -- see _transitive_ancestors from_id, to_id = edge.get("from"), edge.get("to") if from_id in node_ids and to_id in node_ids: for node in nodes: if node.get("node_id") == to_id and from_id not in node["depends_on"]: node["depends_on"].append(from_id) - by_artifact_type = {n.get("artifact_type"): n for n in nodes} - if all(t in by_artifact_type for t in _CANONICAL_DOC_CHAIN): - for prev_type, cur_type in zip(_CANONICAL_DOC_CHAIN, _CANONICAL_DOC_CHAIN[1:]): - prev_id = by_artifact_type[prev_type]["node_id"] - cur_node = by_artifact_type[cur_type] - if prev_id not in cur_node["depends_on"]: - cur_node["depends_on"].append(prev_id) - - # 2026-08-06: test_script's own prompt needs {benchmark_content} - # directly (not just what survives into test_dataset_content, which - # can be a terse 'N/A: ...' with none of the original scenario - # detail) -- the plain linear chain above only gives it - # test_dataset as an immediate dependency, so add benchmark as a - # second one. e2e_pipeline._planner_prompt_builder already threads - # every *immediate* dependency's content through generically, this - # is the one canonical node that genuinely needs two. - benchmark_id = by_artifact_type["benchmark"]["node_id"] - script_node = by_artifact_type["test_script"] - if benchmark_id not in script_node["depends_on"]: - script_node["depends_on"].append(benchmark_id) - + _validate_loop_back_targets(manifest) return manifest @@ -210,14 +337,18 @@ def plan_work_graph(conn, org_node, task_id, goal_text, target_folder_path, time """Returns {"run_id", "graph_manifest"}. Creates one `work_graph_run` row (status='planning') -- moving it to 'running'/executing it is out of scope. - E (2026-08-06): `org_node == 'planner'` still gets the fixed - brd/prd/sdd/benchmark/... chain prompt (_PROMPT_TEMPLATE, unchanged). - Every other org_node gets _GENERIC_PROMPT_TEMPLATE -- the cross-Org- - Agent dynamic planning path described in the design-dynamic-work-graph - Skill, whose nodes carry their own task_prompt instead of a fixed - artifact_type. Function signature/return shape are unchanged either way.""" - template = _PROMPT_TEMPLATE if org_node == "planner" else _GENERIC_PROMPT_TEMPLATE - prompt = template.format(goal_text=goal_text) + 2026-08-08: `org_node == 'planner'` gets `_PROMPT_TEMPLATE` (now a + genuine self-planning prompt over `_PLANNER_NODE_CATALOG`, see that + template's own comment for why goal_text is filled in via `.replace()` + rather than `.format()`). Every other org_node gets + `_GENERIC_PROMPT_TEMPLATE` -- the cross-Org-Agent dynamic planning path + described in the design-dynamic-work-graph Skill, whose nodes carry + their own task_prompt instead of a fixed artifact_type. Function + signature/return shape are unchanged either way.""" + if org_node == "planner": + prompt = _PROMPT_TEMPLATE.replace("{goal_text}", goal_text) + else: + prompt = _GENERIC_PROMPT_TEMPLATE.format(goal_text=goal_text) result = subprocess.run( [ -- libgit2 0.21.4