Commit f701da9d9a4139f886faaf9569a2f67d1145a0ad
1 parent
577d9af0
graphlemon: auto-commit uncommitted changes for 7055a15f-f35a-4b04-a990-8f3fcfb5…
…437e-main-checkout-drift
Showing
6 changed files
with
436 additions
and
57 deletions
docs/todo/work-graph-partial-artifact-persistence-requirements.md
0 → 100644
| 1 | +# Work Graph 产出应逐节点落盘,而不是整图成功才批量落盘——需求文档 | ||
| 2 | + | ||
| 3 | +**日期**: 2026-08-07 | ||
| 4 | +**性质**: 需求文档(BRD/PRD 层面),回应一次真实事故的根因排查——任务 `bfd3ee82`(导航栏功能)和 `b243dfe8`(`#/skills` 页面 bug,已有同目标的 `7fc3a7a3` 成功过)都撞上了同一个真实 bug。 | ||
| 5 | +**状态**: ✅ 已实现(2026-08-07)。 | ||
| 6 | + | ||
| 7 | +## 1. 背景 | ||
| 8 | + | ||
| 9 | +`src/e2e_pipeline.py::plan_documents()` 和 `validate_implementation_with_own_graph()` 都是同一个模式:先调用 `work_graph_orchestrate.run_graph()` 跑完**整张图的全部节点**,拿到返回的 `outputs` 字典后,再用一个 `for node in topological_order(...)` 循环把每个节点的产出批量写成 `artifact` 行。 | ||
| 10 | + | ||
| 11 | +问题:如果图里**任何一个节点**(哪怕是最后一个,比如 `test_script`)在 `run_graph()` 内部失败到耗尽重试(`ConsistencyCheckFailedError`),`run_graph()` 会直接抛异常,`outputs` 这个局部变量随着异常传播被整个丢弃——**哪怕 BRD/PRD/SDD/Benchmark 都已经成功生成,一个 artifact 都不会落盘**。 | ||
| 12 | + | ||
| 13 | +真实复现链路(`bfd3ee82`): | ||
| 14 | +1. Planner 的文档链 BRD→PRD→SDD→Benchmark→TestDataset→TestScript,前五个节点都成功,`TestScript` 节点因为另一个已修复的 bug(`check_script_compiles` 返回裸字符串导致 `join()` 逐字符拆解)连续两次验证失败,`run_graph()` 抛 `ConsistencyCheckFailedError`。 | ||
| 15 | +2. 异常一路传到 `scheduler.py`/`task_execution.py` 的兜底处理,`task.status` 变成 `blocked_pending_human`,但此时**没有任何 artifact 被写入**。 | ||
| 16 | +3. 人工点 Resume,`task.status` 恢复到之前记录的 `planning_brd_prd_sdd`(这是 Planner 的 POST-state,语义是"Planner 已完成,该轮到 Coder 了")。 | ||
| 17 | +4. `task_execution.py::run_or_resume_task_lifecycle()` 的 `planning_brd_prd_sdd` 分支**直接读取 SDD/PRD/Benchmark artifact 内容准备交给 Coder**——但因为第 2 步根本没写任何 artifact,`_artifact_content(..., "sdd")` 查不到行,抛 `TaskExecutionError("task ... has no sdd artifact to resume from")`,任务再次卡死,且这次连"重跑一次 Planner"都做不到(resume 逻辑假设走到这个状态时 Planner 一定已经把 artifact 写好了)。 | ||
| 18 | + | ||
| 19 | +`b243dfe8` 是同一个根因的更早一次复现(当时判断为"陈旧重复项",实际是同一个 bug,只是恰好有另一个任务 `7fc3a7a3` 后来重跑成功了,掩盖了这不是任务本身的问题)。 | ||
| 20 | + | ||
| 21 | +## 2. 目标 | ||
| 22 | + | ||
| 23 | +1. Work Graph 里每个节点**成功完成后立即落盘为 artifact**,不再等整张图全部成功才批量写。 | ||
| 24 | +2. 一张图里某个节点失败(重试耗尽)时,**已经成功的节点产出必须已经落盘**——任务变成 `blocked_pending_human` 后,人工点 Resume 应该能利用这些已落盘的产出继续,而不是从零开始、也不是像现在这样连"没有 artifact"这种最基础的前提都保证不了。 | ||
| 25 | +3. 覆盖两个已知有这个问题的调用点:`plan_documents()`(Planner 的 BRD→PRD→SDD→Benchmark→TestDataset→TestScript 链)和 `validate_implementation_with_own_graph()`(Validator 自己的 benchmark→test_dataset→test_script 链)——两处是同一个模式复制出来的,应该用同一个修复方式,不要修一处漏一处。 | ||
| 26 | + | ||
| 27 | +## 3. 非目标 | ||
| 28 | + | ||
| 29 | +1. **不改 `work_graph_orchestrate.run_graph()` 本身的执行逻辑**——那是 G(并行执行)刚重做过的核心路径,改动面和回归风险都更大。这次修复只动 `plan_documents()`/`validate_implementation_with_own_graph()` 这两个调用方自己怎么处理 `run_graph()` 的返回值/异常,不碰 `run_graph()` 内部。 | ||
| 30 | +2. **不要求"部分失败"之后自动决定要不要继续跑剩下的节点**——那是失败处理策略的问题(比如要不要自动重试整张图剩下的部分),本需求只解决"已经成功的部分有没有被记住"这一件事,继续不继续是既有的 Org-level Loop/人工 Resume 机制的职责,不重新设计。 | ||
| 31 | +3. **不改变任务失败后的对外状态语义**——图里有节点失败,任务依然应该变成 `blocked_pending_human`,不是这次改动想让它"看起来像成功了"。只是失败之后,已经做对的那部分工作不应该被扔掉。 | ||
| 32 | + | ||
| 33 | +## 4. 功能需求 | ||
| 34 | + | ||
| 35 | +| # | 需求描述 | 优先级 | | ||
| 36 | +|---|---|---| | ||
| 37 | +| FR-1 | `plan_documents()`/`validate_implementation_with_own_graph()` 里"把节点产出写成 artifact"这一步,数据来源要改成直接查 `work_node_execution` 表里 `status='done'` 的行(`output_payload` 已经是 REQ-06 断点续跑机制本来就会持久化的字段,不是新数据源),而不是依赖 `run_graph()` 成功返回后的内存 `outputs` 字典——这样无论 `run_graph()` 是正常返回还是中途抛异常,只要节点在 DB 里已经是 `done`,就能被正确落盘。 | 必须 | | ||
| 38 | +| FR-2 | `run_graph()` 抛出的异常(`ConsistencyCheckFailedError` 等)在完成 FR-1 的落盘之后必须原样重新抛出——不能吞掉,任务该进 `blocked_pending_human` 还是要进,这次修复不改变失败语义(见非目标 3)。 | 必须 | | ||
| 39 | +| FR-3 | 落盘循环要能正确跳过"没跑到/没成功"的节点,不能因为某个节点没有 `output_payload` 就整个循环报错——已有的 `deps` 计算逻辑("依赖没落盘就不算进 deps 里")本来就是为这种情况设计的,复用它,不要重新发明。 | 必须 | | ||
| 40 | +| FR-4 | Planner 和 Validator 两处调用点要复用同一份"按 DB 里的 done 节点落盘"辅助逻辑,不要各写一份——两处现在的写法本来就是复制出来的,统一之后以后再出现第三处类似场景也有地方复用。 | 应该 | | ||
| 41 | + | ||
| 42 | +## 5. 验收标准 | ||
| 43 | + | ||
| 44 | +1. 人为构造一个"前面节点成功、最后一个节点连续验证失败"的场景(复现 `bfd3ee82` 的真实序列),验证 `run_graph()` 抛异常之后,前面成功节点对应的 artifact 确实已经写入 `artifact` 表,且原始异常被正确重新抛出(调用方拿到的还是 `ConsistencyCheckFailedError`,不是别的)。 | ||
| 45 | +2. 验证正常全部成功的路径行为不变——既有测试套件不需要修改断言就应该继续通过。 | ||
| 46 | +3. 验证 Planner 侧修好之后,一个曾经因为这个 bug 卡死的任务,点 Resume 后不会再报"没有 SDD artifact 可恢复"。 | ||
| 47 | +4. Validator 侧同样验证一遍(构造一个 Validator 自己的 benchmark→test_dataset→test_script 链中途失败的场景)。 | ||
| 48 | + | ||
| 49 | +## 6. 约束与假设 | ||
| 50 | + | ||
| 51 | +- 复用 REQ-06 已经建立的"`work_node_execution.output_payload` 是节点成功后的权威持久化数据"这个既有约定,不引入新的数据来源或新的持久化机制。 | ||
| 52 | +- 这次修复完全在 `e2e_pipeline.py` 内部(两个调用方函数),不涉及 `work_graph_orchestrate.py`/`work_node_run.py` 的任何改动。 | ||
| 53 | + | ||
| 54 | +## 7. 已实现 | ||
| 55 | + | ||
| 56 | +新增 `src/e2e_pipeline.py::_write_completed_node_artifacts()`,`plan_documents()`/`validate_implementation_with_own_graph()` 都改成:`run_graph()` 包一层 `try/except`,无论成功还是失败都调用这个共享辅助函数落盘,失败时原始异常原样重新抛出。 | ||
| 57 | + | ||
| 58 | +**实现过程中发现的一个比原设计更细的坑,FR-1 原文没写清楚**:`work_node_execution.status='done'` 只代表"通过了 schema 校验",不代表"通过了 `verify_fn` 的语义检查"——一个节点被 `verify_fn` 拒绝、重试一次后仍然被拒绝(`ConsistencyCheckFailedError`),它自己和它的"-reflect"重试节点在 DB 里其实都是 `status='done'`(只是内容都不合格)。如果单纯按"状态是不是 done"来判断能不能落盘,会把**verify_fn 明确拒绝的内容**也当成"已完成"错误地写成 artifact。修复方式:给 `ConsistencyCheckFailedError` 加了 `node_id` 属性(`work_graph_orchestrate.py`),`_write_completed_node_artifacts` 用它(以及 `WorkGraphLayerError.errors`,覆盖未来多节点并行层的等价场景)排除掉这些节点,不只是排除"根本没跑完"的节点。 | ||
| 59 | + | ||
| 60 | +同时顺带修了一个只有在这次实现验证时才会暴露的遗留 bug:`validate_implementation_with_own_graph()` 早些时候那次"结束时标记 `work_graph_run.status='done'`"的修复,只覆盖了成功路径——如果 `run_graph()` 本身抛出的不是 `ConsistencyCheckFailedError`(那个走的是 `_verify_and_maybe_regenerate` 自己的 `status='blocked'` 写入,已经安全),而是别的异常类型,`work_graph_run` 还是会卡在 `running`。这次统一走 `_write_completed_node_artifacts` 之后,失败路径已经不需要再单独处理这个——`_verify_and_maybe_regenerate` 自己的 `blocked` 写入本来就覆盖了目前唯一真实会走到这条路径的异常类型,测试里专门加了一条断言确认这一点没有回归。 | ||
| 61 | + | ||
| 62 | +新增测试:`src/test_e2e_pipeline.py` 的 `PlanDocumentsTest.test_earlier_nodes_survive_a_later_node_failing_verification`、`ValidateImplementationWithOwnGraphTest.test_earlier_nodes_survive_a_later_node_failing_verification`——都验证"前面节点产出确实落盘、失败节点本身不落盘、原始异常原样抛出"。`python3 -m unittest discover -s src`(641 tests)+ `discover -s supervisor -p "test_*.py"`(126 tests)全绿,零回归。 |
src/backend_server.py
| @@ -540,8 +540,16 @@ class _Handler(BaseHTTPRequestHandler): | @@ -540,8 +540,16 @@ class _Handler(BaseHTTPRequestHandler): | ||
| 540 | except (org_loop_route.TaskNotBlockedError, org_loop_route.TaskNotValidatingError, ValueError) as e: | 540 | except (org_loop_route.TaskNotBlockedError, org_loop_route.TaskNotValidatingError, ValueError) as e: |
| 541 | self._send_json(400, {"error": "INVALID_PARAM", "detail": str(e)}) | 541 | self._send_json(400, {"error": "INVALID_PARAM", "detail": str(e)}) |
| 542 | return | 542 | return |
| 543 | + # 'validating' added 2026-08-07 (real incident): task_execution. | ||
| 544 | + # run_or_resume_task_lifecycle has resumed from 'validating' since | ||
| 545 | + # the earlier same-day fix (skips straight to | ||
| 546 | + # validate_implementation_with_own_graph, the readiness trigger | ||
| 547 | + # already having run once) -- but THIS allowlist, a separate code | ||
| 548 | + # path, still didn't know that, so a manual Resume on a task stuck | ||
| 549 | + # in 'validating' flipped the status back with nothing ever | ||
| 550 | + # actually continuing it. | ||
| 543 | if pre_resume_reason not in ("budget_exceeded", "loop_limit_reached") and result.get("new_status") in ( | 551 | if pre_resume_reason not in ("budget_exceeded", "loop_limit_reached") and result.get("new_status") in ( |
| 544 | - "queued", "planning_brd_prd_sdd", "coding", | 552 | + "queued", "planning_brd_prd_sdd", "coding", "validating", |
| 545 | ): | 553 | ): |
| 546 | task_row = self.server.conn.execute( | 554 | task_row = self.server.conn.execute( |
| 547 | "SELECT project_id, goal_text FROM task WHERE task_id = ?", (task_id,) | 555 | "SELECT project_id, goal_text FROM task WHERE task_id = ?", (task_id,) |
| @@ -549,9 +557,17 @@ class _Handler(BaseHTTPRequestHandler): | @@ -549,9 +557,17 @@ class _Handler(BaseHTTPRequestHandler): | ||
| 549 | target_folder_path = self.server.conn.execute( | 557 | target_folder_path = self.server.conn.execute( |
| 550 | "SELECT target_folder_path FROM project WHERE project_id = ?", (task_row[0],) | 558 | "SELECT target_folder_path FROM project WHERE project_id = ?", (task_row[0],) |
| 551 | ).fetchone()[0] | 559 | ).fetchone()[0] |
| 552 | - task_execution.start_task_execution_in_background( | ||
| 553 | - self.server.db_path, task_id, target_folder_path, task_row[1], | ||
| 554 | - ) | 560 | + try: |
| 561 | + task_execution.start_task_execution_in_background( | ||
| 562 | + self.server.db_path, task_id, target_folder_path, task_row[1], | ||
| 563 | + ) | ||
| 564 | + except task_execution.TaskExecutionError: | ||
| 565 | + # Already has an execution in flight (e.g. the | ||
| 566 | + # reconciliation loop dispatched it a moment before this | ||
| 567 | + # click landed) -- the resume itself already succeeded | ||
| 568 | + # (route_org_loop.route_org_loop above), so still report | ||
| 569 | + # 200; the in-flight run will get there on its own. | ||
| 570 | + pass | ||
| 555 | self._send_json(200, result) | 571 | self._send_json(200, result) |
| 556 | 572 | ||
| 557 | def _handle_task_execute(self, task_id): | 573 | def _handle_task_execute(self, task_id): |
| @@ -572,7 +588,14 @@ class _Handler(BaseHTTPRequestHandler): | @@ -572,7 +588,14 @@ class _Handler(BaseHTTPRequestHandler): | ||
| 572 | target_folder_path = self.server.conn.execute( | 588 | target_folder_path = self.server.conn.execute( |
| 573 | "SELECT target_folder_path FROM project WHERE project_id = ?", (project_id,) | 589 | "SELECT target_folder_path FROM project WHERE project_id = ?", (project_id,) |
| 574 | ).fetchone()[0] | 590 | ).fetchone()[0] |
| 575 | - task_execution.start_task_execution_in_background(self.server.db_path, task_id, target_folder_path, goal_text) | 591 | + try: |
| 592 | + task_execution.start_task_execution_in_background(self.server.db_path, task_id, target_folder_path, goal_text) | ||
| 593 | + except task_execution.TaskExecutionError as e: | ||
| 594 | + # 2026-08-07: already has an execution in flight (e.g. a rapid | ||
| 595 | + # double-click, or the reconciliation loop won a race against | ||
| 596 | + # this click) -- report it instead of a 500. | ||
| 597 | + self._send_json(409, {"error": "ALREADY_IN_FLIGHT", "detail": str(e)}) | ||
| 598 | + return | ||
| 576 | self._send_json(200, {"task_id": task_id, "started": True}) | 599 | self._send_json(200, {"task_id": task_id, "started": True}) |
| 577 | 600 | ||
| 578 | def _handle_task_pause(self, task_id, reason): | 601 | def _handle_task_pause(self, task_id, reason): |
src/e2e_pipeline.py
| @@ -439,12 +439,106 @@ def _combined_planner_verify_fn(node_artifact_types, target_folder_path): | @@ -439,12 +439,106 @@ def _combined_planner_verify_fn(node_artifact_types, target_folder_path): | ||
| 439 | return verify_fn | 439 | return verify_fn |
| 440 | 440 | ||
| 441 | 441 | ||
| 442 | +def _failed_node_ids(exc): | ||
| 443 | + """2026-08-07: which node_id(s) a run_graph() failure means were | ||
| 444 | + NEVER verify_fn-accepted, even if their work_node_execution row is | ||
| 445 | + 'done' (schema-valid only -- see ConsistencyCheckFailedError's own | ||
| 446 | + docstring). Handles both the common case today (a bare | ||
| 447 | + ConsistencyCheckFailedError -- Planner's/Validator's doc chains are | ||
| 448 | + currently serial, one node per layer) and WorkGraphLayerError (G's | ||
| 449 | + multi-node-layer aggregate, for whenever a fan-out graph hits this).""" | ||
| 450 | + if isinstance(exc, work_graph_orchestrate.WorkGraphLayerError): | ||
| 451 | + return {node_id for node_id, _ in exc.errors} | ||
| 452 | + if isinstance(exc, work_graph_orchestrate.ConsistencyCheckFailedError) and exc.node_id: | ||
| 453 | + return {exc.node_id} | ||
| 454 | + return set() | ||
| 455 | + | ||
| 456 | + | ||
| 457 | +def _write_completed_node_artifacts( | ||
| 458 | + conn, target_folder_path, task_id, run_id, graph_manifest, node_artifact_types, ext_for, run_graph_error=None, | ||
| 459 | +): | ||
| 460 | + """Writes an artifact for every node that's genuinely done AND | ||
| 461 | + verify_fn-accepted, sourced from `work_node_execution` in the DB -- | ||
| 462 | + not a run_graph() return value, so this works identically whether | ||
| 463 | + run_graph() returned normally (every node accounted for) or raised | ||
| 464 | + partway through (only some nodes are). | ||
| 465 | + | ||
| 466 | + 2026-08-07 (real incident, docs/todo/work-graph-partial-artifact- | ||
| 467 | + persistence-requirements.md): run_graph() raising discards its | ||
| 468 | + would-be return value entirely (a Python exception drops the local | ||
| 469 | + scope), so previously ANY node failing -- even the very last one in | ||
| 470 | + an otherwise-fully-successful chain -- meant NOTHING got persisted, | ||
| 471 | + silently throwing away real, already-generated, already-good content. | ||
| 472 | + A resumed task then found no BRD/PRD/SDD artifacts to build on at all, | ||
| 473 | + even though they'd been generated correctly the first time. | ||
| 474 | + | ||
| 475 | + A node whose OWN "-reflect" retry exists (verify_fn rejected the | ||
| 476 | + first attempt) uses the reflect attempt's content, not the original's | ||
| 477 | + -- the reflect node_id isn't part of `graph_manifest` so the main loop | ||
| 478 | + below never visits it directly. `_failed_node_ids(run_graph_error)` | ||
| 479 | + nodes are excluded outright: their row is schema-valid ('done') but | ||
| 480 | + verify_fn never accepted either attempt, so neither is trustworthy | ||
| 481 | + content to persist. | ||
| 482 | + """ | ||
| 483 | + done_rows = conn.execute( | ||
| 484 | + "SELECT node_id, output_payload FROM work_node_execution WHERE run_id = ? AND status = 'done'", | ||
| 485 | + (run_id,), | ||
| 486 | + ).fetchall() | ||
| 487 | + done_outputs = {node_id: json.loads(output_payload) for node_id, output_payload in done_rows} | ||
| 488 | + for node_id in list(done_outputs): | ||
| 489 | + reflect_id = node_id + "-reflect" | ||
| 490 | + if reflect_id in done_outputs: | ||
| 491 | + done_outputs[node_id] = done_outputs[reflect_id] | ||
| 492 | + for excluded in _failed_node_ids(run_graph_error): | ||
| 493 | + done_outputs.pop(excluded, None) | ||
| 494 | + | ||
| 495 | + artifact_ids = {} | ||
| 496 | + for node in work_graph_orchestrate.topological_order(graph_manifest): | ||
| 497 | + node_id = node["node_id"] | ||
| 498 | + if node_id not in done_outputs: | ||
| 499 | + continue | ||
| 500 | + artifact_type = node_artifact_types[node_id] | ||
| 501 | + deps = [ | ||
| 502 | + artifact_ids[node_artifact_types[dep_id]] | ||
| 503 | + for dep_id in (node.get("depends_on") or []) | ||
| 504 | + if node_artifact_types.get(dep_id) in artifact_ids | ||
| 505 | + ] | ||
| 506 | + result = artifact_write.write_artifact( | ||
| 507 | + conn, target_folder_path, artifact_type, task_id, run_id, | ||
| 508 | + ".graphlemon/runs/{}/{}.{}".format(task_id, artifact_type, ext_for(artifact_type)), | ||
| 509 | + done_outputs[node_id]["content"], depends_on_artifact_ids=deps, | ||
| 510 | + ) | ||
| 511 | + artifact_ids[artifact_type] = result["artifact_id"] | ||
| 512 | + return artifact_ids | ||
| 513 | + | ||
| 514 | + | ||
| 515 | +# test_dataset/test_script aren't necessarily Markdown (a script is real | ||
| 516 | +# code, a dataset could be JSON/CSV/anything) -- .txt is a deliberately | ||
| 517 | +# generic container; Coder materializes it into a properly-named, | ||
| 518 | +# properly-placed real file under the target project's own test | ||
| 519 | +# conventions when it actually uses it, that's not this write step's job. | ||
| 520 | +_PLANNER_ARTIFACT_FILE_EXT = {"test_dataset": "txt", "test_script": "txt"} | ||
| 521 | + | ||
| 522 | + | ||
| 523 | +def _planner_ext_for(artifact_type): | ||
| 524 | + return _PLANNER_ARTIFACT_FILE_EXT.get(artifact_type, "md") | ||
| 525 | + | ||
| 526 | + | ||
| 442 | def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | 527 | def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): |
| 443 | """Architecture §11 criterion 1: a real Planner Work Graph | 528 | """Architecture §11 criterion 1: a real Planner Work Graph |
| 444 | (c_work_graph_plan -> per-node c_work_graph_execute/c_work_node_run via | 529 | (c_work_graph_plan -> per-node c_work_graph_execute/c_work_node_run via |
| 445 | work_graph_orchestrate.run_graph) producing BRD->PRD->SDD->Benchmark, | 530 | work_graph_orchestrate.run_graph) producing BRD->PRD->SDD->Benchmark, |
| 446 | each written via c_artifact_write with correct lineage. | 531 | each written via c_artifact_write with correct lineage. |
| 447 | Returns {"run_id": ..., "artifact_ids": {artifact_type: artifact_id}}. | 532 | Returns {"run_id": ..., "artifact_ids": {artifact_type: artifact_id}}. |
| 533 | + | ||
| 534 | + 2026-08-07: artifacts are now written from whatever's genuinely done in | ||
| 535 | + the DB (see _write_completed_node_artifacts), not from run_graph()'s | ||
| 536 | + in-memory return value -- so a failure partway through (e.g. the last | ||
| 537 | + node in the chain) still persists everything that succeeded before it, | ||
| 538 | + instead of discarding it all. The original run_graph() failure is | ||
| 539 | + re-raised unchanged after salvaging -- this does not change whether or | ||
| 540 | + when a task ends up blocked_pending_human, only what's left behind for | ||
| 541 | + a human's Resume to build on when it does. | ||
| 448 | """ | 542 | """ |
| 449 | plan_result = work_graph_plan.plan_work_graph( | 543 | plan_result = work_graph_plan.plan_work_graph( |
| 450 | conn, "planner", task_id, goal_text, target_folder_path, timeout=timeout | 544 | conn, "planner", task_id, goal_text, target_folder_path, timeout=timeout |
| @@ -453,35 +547,24 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | @@ -453,35 +547,24 @@ def plan_documents(conn, task_id, goal_text, target_folder_path, timeout=300): | ||
| 453 | graph_manifest = plan_result["graph_manifest"] | 547 | graph_manifest = plan_result["graph_manifest"] |
| 454 | node_artifact_types = {n["node_id"]: n.get("artifact_type", n["node_id"]) for n in graph_manifest["nodes"]} | 548 | node_artifact_types = {n["node_id"]: n.get("artifact_type", n["node_id"]) for n in graph_manifest["nodes"]} |
| 455 | 549 | ||
| 456 | - outputs = work_graph_orchestrate.run_graph( | ||
| 457 | - conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, | ||
| 458 | - _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, | ||
| 459 | - verify_fn=_combined_planner_verify_fn(node_artifact_types, target_folder_path), | 550 | + run_graph_error = None |
| 551 | + try: | ||
| 552 | + work_graph_orchestrate.run_graph( | ||
| 553 | + conn, run_id, "planner", "planner-doc-writing", target_folder_path, graph_manifest, | ||
| 554 | + _planner_prompt_builder(goal_text, node_artifact_types), _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, | ||
| 555 | + verify_fn=_combined_planner_verify_fn(node_artifact_types, target_folder_path), | ||
| 556 | + ) | ||
| 557 | + except Exception as e: | ||
| 558 | + run_graph_error = e | ||
| 559 | + | ||
| 560 | + artifact_ids = _write_completed_node_artifacts( | ||
| 561 | + conn, target_folder_path, task_id, run_id, graph_manifest, node_artifact_types, | ||
| 562 | + _planner_ext_for, run_graph_error, | ||
| 460 | ) | 563 | ) |
| 461 | 564 | ||
| 462 | - # test_dataset/test_script aren't necessarily Markdown (a script is real | ||
| 463 | - # code, a dataset could be JSON/CSV/anything) -- .txt is a deliberately | ||
| 464 | - # generic container; Coder materializes it into a properly-named, | ||
| 465 | - # properly-placed real file under the target project's own test | ||
| 466 | - # conventions when it actually uses it, that's not this write step's job. | ||
| 467 | - _ARTIFACT_FILE_EXT = {"test_dataset": "txt", "test_script": "txt"} | 565 | + if run_graph_error is not None: |
| 566 | + raise run_graph_error | ||
| 468 | 567 | ||
| 469 | - artifact_ids = {} | ||
| 470 | - for node in work_graph_orchestrate.topological_order(graph_manifest): | ||
| 471 | - node_id = node["node_id"] | ||
| 472 | - artifact_type = node_artifact_types[node_id] | ||
| 473 | - deps = [ | ||
| 474 | - artifact_ids[node_artifact_types[dep_id]] | ||
| 475 | - for dep_id in (node.get("depends_on") or []) | ||
| 476 | - if node_artifact_types.get(dep_id) in artifact_ids | ||
| 477 | - ] | ||
| 478 | - ext = _ARTIFACT_FILE_EXT.get(artifact_type, "md") | ||
| 479 | - result = artifact_write.write_artifact( | ||
| 480 | - conn, target_folder_path, artifact_type, task_id, run_id, | ||
| 481 | - ".graphlemon/runs/{}/{}.{}".format(task_id, artifact_type, ext), outputs[node_id]["content"], | ||
| 482 | - depends_on_artifact_ids=deps, | ||
| 483 | - ) | ||
| 484 | - artifact_ids[artifact_type] = result["artifact_id"] | ||
| 485 | return {"run_id": run_id, "artifact_ids": artifact_ids} | 568 | return {"run_id": run_id, "artifact_ids": artifact_ids} |
| 486 | 569 | ||
| 487 | 570 | ||
| @@ -663,30 +746,32 @@ def validate_implementation_with_own_graph( | @@ -663,30 +746,32 @@ def validate_implementation_with_own_graph( | ||
| 663 | conn.commit() | 746 | conn.commit() |
| 664 | 747 | ||
| 665 | node_artifact_types = {n["node_id"]: n["artifact_type"] for n in _VALIDATOR_MANIFEST["nodes"]} | 748 | node_artifact_types = {n["node_id"]: n["artifact_type"] for n in _VALIDATOR_MANIFEST["nodes"]} |
| 666 | - outputs = work_graph_orchestrate.run_graph( | ||
| 667 | - conn, run_id, "validator", "validator-independent-verification", target_folder_path, _VALIDATOR_MANIFEST, | ||
| 668 | - _validator_prompt_builder(goal_text, sdd_content, prd_content, node_artifact_types), | ||
| 669 | - _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, | ||
| 670 | - verify_fn=_test_script_verify_fn(node_artifact_types, script_artifact_type="validator_test_script"), | ||
| 671 | - timeout=timeout, | 749 | + # 2026-08-07: same run_graph()-partial-failure salvage as |
| 750 | + # plan_documents() -- see _write_completed_node_artifacts's own | ||
| 751 | + # docstring and docs/todo/work-graph-partial-artifact-persistence- | ||
| 752 | + # requirements.md. `outputs` is only used below on the SUCCESS | ||
| 753 | + # continuation (script execution + verdict prompt); the failure path | ||
| 754 | + # re-raises before ever touching it. | ||
| 755 | + run_graph_error = None | ||
| 756 | + outputs = None | ||
| 757 | + try: | ||
| 758 | + outputs = work_graph_orchestrate.run_graph( | ||
| 759 | + conn, run_id, "validator", "validator-independent-verification", target_folder_path, _VALIDATOR_MANIFEST, | ||
| 760 | + _validator_prompt_builder(goal_text, sdd_content, prd_content, node_artifact_types), | ||
| 761 | + _DOC_INPUT_SCHEMA, _DOC_OUTPUT_SCHEMA, | ||
| 762 | + verify_fn=_test_script_verify_fn(node_artifact_types, script_artifact_type="validator_test_script"), | ||
| 763 | + timeout=timeout, | ||
| 764 | + ) | ||
| 765 | + except Exception as e: | ||
| 766 | + run_graph_error = e | ||
| 767 | + | ||
| 768 | + artifact_ids = _write_completed_node_artifacts( | ||
| 769 | + conn, target_folder_path, task_id, run_id, _VALIDATOR_MANIFEST, node_artifact_types, | ||
| 770 | + lambda artifact_type: "txt" if artifact_type != "validator_benchmark" else "md", run_graph_error, | ||
| 672 | ) | 771 | ) |
| 673 | 772 | ||
| 674 | - artifact_ids = {} | ||
| 675 | - for node in work_graph_orchestrate.topological_order(_VALIDATOR_MANIFEST): | ||
| 676 | - node_id = node["node_id"] | ||
| 677 | - artifact_type = node_artifact_types[node_id] | ||
| 678 | - deps = [ | ||
| 679 | - artifact_ids[node_artifact_types[dep_id]] | ||
| 680 | - for dep_id in (node.get("depends_on") or []) | ||
| 681 | - if node_artifact_types.get(dep_id) in artifact_ids | ||
| 682 | - ] | ||
| 683 | - ext = "txt" if artifact_type != "validator_benchmark" else "md" | ||
| 684 | - result = artifact_write.write_artifact( | ||
| 685 | - conn, target_folder_path, artifact_type, task_id, run_id, | ||
| 686 | - ".graphlemon/runs/{}/{}.{}".format(task_id, artifact_type, ext), outputs[node_id]["content"], | ||
| 687 | - depends_on_artifact_ids=deps, | ||
| 688 | - ) | ||
| 689 | - artifact_ids[artifact_type] = result["artifact_id"] | 773 | + if run_graph_error is not None: |
| 774 | + raise run_graph_error | ||
| 690 | 775 | ||
| 691 | script_content = outputs["validator_test_script"]["content"] | 776 | script_content = outputs["validator_test_script"]["content"] |
| 692 | script_execution = run_test_script_for_real( | 777 | script_execution = run_test_script_for_real( |
| @@ -711,6 +796,21 @@ def validate_implementation_with_own_graph( | @@ -711,6 +796,21 @@ def validate_implementation_with_own_graph( | ||
| 711 | if script_execution["ran"] and script_execution["exit_code"] not in (0, None): | 796 | if script_execution["ran"] and script_execution["exit_code"] not in (0, None): |
| 712 | verdict = "FAIL" | 797 | verdict = "FAIL" |
| 713 | 798 | ||
| 799 | + # 2026-08-07 (real incident): this function INSERTs its work_graph_run | ||
| 800 | + # row as status='running' at the top and, on the happy path (verify_fn | ||
| 801 | + # returns ok=True, so _verify_and_maybe_regenerate never touches | ||
| 802 | + # work_graph_run.status at all -- it only writes 'reflecting'/ | ||
| 803 | + # 'verifying'/'blocked' on the RETRY path), never wrote anything past | ||
| 804 | + # that -- regardless of PASS/FAIL/INCONCLUSIVE, a validation run that | ||
| 805 | + # got this far genuinely finished executing. `ux_work_graph_run_active` | ||
| 806 | + # (schema.py) is a unique index on (task_id, org_node) covering exactly | ||
| 807 | + # this non-terminal status set, so a real re-verify later (org_level_ | ||
| 808 | + # loop.run_fix_loop after a FAIL) collided with this row on INSERT -- | ||
| 809 | + # "UNIQUE constraint failed: work_graph_run.task_id, work_graph_run. | ||
| 810 | + # org_node" -- found for real via a task reconciliation resume. | ||
| 811 | + conn.execute("UPDATE work_graph_run SET status = 'done' WHERE run_id = ?", (run_id,)) | ||
| 812 | + conn.commit() | ||
| 813 | + | ||
| 714 | return { | 814 | return { |
| 715 | "verdict": verdict, "raw_response": response, | 815 | "verdict": verdict, "raw_response": response, |
| 716 | "artifact_ids": artifact_ids, "script_execution": script_execution, | 816 | "artifact_ids": artifact_ids, "script_execution": script_execution, |
src/test_backend_server.py
| @@ -874,6 +874,59 @@ class TaskExecutionRouteTest(unittest.TestCase): | @@ -874,6 +874,59 @@ class TaskExecutionRouteTest(unittest.TestCase): | ||
| 874 | self.assertEqual(status, 200) | 874 | self.assertEqual(status, 200) |
| 875 | mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal") | 875 | mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal") |
| 876 | 876 | ||
| 877 | + def test_manual_resume_from_validating_auto_continues(self): | ||
| 878 | + # 2026-08-07 real incident: a task stuck in 'validating' (the | ||
| 879 | + # readiness trigger already ran once -- see task_execution.py's | ||
| 880 | + # 'validating' resume section) got its status flipped back by | ||
| 881 | + # manual_resume but nothing ever actually continued it, because | ||
| 882 | + # this allowlist hadn't been updated to match task_execution. | ||
| 883 | + # run_or_resume_task_lifecycle's own 'validating' support added | ||
| 884 | + # earlier the same day. | ||
| 885 | + self.server.conn.execute( | ||
| 886 | + "UPDATE task SET status = 'blocked_pending_human', blocked_reason = 'execution_error' " | ||
| 887 | + "WHERE task_id = 'task-1'" | ||
| 888 | + ) | ||
| 889 | + self.server.conn.execute( | ||
| 890 | + "INSERT INTO execution_event (event_id, project_id, task_id, event_type, payload) " | ||
| 891 | + "VALUES ('evt-1', 'proj-1', 'task-1', 'task_status_changed', " | ||
| 892 | + "'{\"from_status\": \"validating\", \"to_status\": \"blocked_pending_human\"}')" | ||
| 893 | + ) | ||
| 894 | + self.server.conn.commit() | ||
| 895 | + with patch.object(backend_server.task_execution, "start_task_execution_in_background") as mock_start: | ||
| 896 | + status, body = self._post("/api/tasks/task-1/route", {"decided_route": "manual_resume"}) | ||
| 897 | + self.assertEqual(status, 200) | ||
| 898 | + mock_start.assert_called_once_with(self.server.db_path, "task-1", "/tmp/proj-1", "the goal") | ||
| 899 | + | ||
| 900 | + def test_execute_already_in_flight_returns_409_not_500(self): | ||
| 901 | + with patch.object( | ||
| 902 | + backend_server.task_execution, "start_task_execution_in_background", | ||
| 903 | + side_effect=backend_server.task_execution.TaskExecutionError("task task-1 already has an execution in flight"), | ||
| 904 | + ): | ||
| 905 | + status, body = self._post("/api/tasks/task-1/execute") | ||
| 906 | + self.assertEqual(status, 409) | ||
| 907 | + self.assertEqual(body["error"], "ALREADY_IN_FLIGHT") | ||
| 908 | + | ||
| 909 | + def test_manual_resume_still_returns_200_when_already_in_flight(self): | ||
| 910 | + # The route_org_loop transition itself already succeeded; losing a | ||
| 911 | + # race to dispatch (e.g. the reconciliation loop got there a moment | ||
| 912 | + # earlier) isn't a failure of the resume request itself. | ||
| 913 | + self.server.conn.execute( | ||
| 914 | + "UPDATE task SET status = 'blocked_pending_human', blocked_reason = 'execution_error' " | ||
| 915 | + "WHERE task_id = 'task-1'" | ||
| 916 | + ) | ||
| 917 | + self.server.conn.execute( | ||
| 918 | + "INSERT INTO execution_event (event_id, project_id, task_id, event_type, payload) " | ||
| 919 | + "VALUES ('evt-1', 'proj-1', 'task-1', 'task_status_changed', " | ||
| 920 | + "'{\"from_status\": \"coding\", \"to_status\": \"blocked_pending_human\"}')" | ||
| 921 | + ) | ||
| 922 | + self.server.conn.commit() | ||
| 923 | + with patch.object( | ||
| 924 | + backend_server.task_execution, "start_task_execution_in_background", | ||
| 925 | + side_effect=backend_server.task_execution.TaskExecutionError("already in flight"), | ||
| 926 | + ): | ||
| 927 | + status, body = self._post("/api/tasks/task-1/route", {"decided_route": "manual_resume"}) | ||
| 928 | + self.assertEqual(status, 200) | ||
| 929 | + | ||
| 877 | 930 | ||
| 878 | def _ws_handshake_request(path, key): | 931 | def _ws_handshake_request(path, key): |
| 879 | return ( | 932 | return ( |
src/test_e2e_pipeline.py
| @@ -29,7 +29,7 @@ import tempfile | @@ -29,7 +29,7 @@ import tempfile | ||
| 29 | import unittest | 29 | import unittest |
| 30 | from unittest.mock import patch | 30 | from unittest.mock import patch |
| 31 | 31 | ||
| 32 | -from src import e2e_pipeline, org_node_trigger, project_scaffold, schema, work_graph_execute | 32 | +from src import e2e_pipeline, org_node_trigger, project_scaffold, schema, work_graph_execute, work_graph_orchestrate |
| 33 | 33 | ||
| 34 | _FAKE_DOCS = { | 34 | _FAKE_DOCS = { |
| 35 | "brd": "# BRD\nfake", "prd": "# PRD\nfake", "sdd": "# SDD\nfake", "benchmark": "# Benchmark\nfake", | 35 | "brd": "# BRD\nfake", "prd": "# PRD\nfake", "sdd": "# SDD\nfake", "benchmark": "# Benchmark\nfake", |
| @@ -120,6 +120,58 @@ class PlanDocumentsTest(PipelineTestBase): | @@ -120,6 +120,58 @@ class PlanDocumentsTest(PipelineTestBase): | ||
| 120 | ).fetchone() | 120 | ).fetchone() |
| 121 | self.assertEqual(run_row, ("planner", "planning")) | 121 | self.assertEqual(run_row, ("planner", "planning")) |
| 122 | 122 | ||
| 123 | + def test_earlier_nodes_survive_a_later_node_failing_verification(self): | ||
| 124 | + # 2026-08-07 real incident (docs/todo/work-graph-partial-artifact- | ||
| 125 | + # persistence-requirements.md): test_script fails verify_fn twice | ||
| 126 | + # (original + one regeneration) -- ConsistencyCheckFailedError must | ||
| 127 | + # still propagate (failure semantics unchanged), but BRD/PRD/SDD/ | ||
| 128 | + # Benchmark/TestDataset -- all genuinely produced and accepted -- | ||
| 129 | + # must be persisted as artifacts, not silently discarded. | ||
| 130 | + self.conn.execute( | ||
| 131 | + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " | ||
| 132 | + "VALUES ('task-1', (SELECT project_id FROM project), 'goal', 'goal', " | ||
| 133 | + "'planning_brd_prd_sdd', 'planner')" | ||
| 134 | + ) | ||
| 135 | + self.conn.commit() | ||
| 136 | + | ||
| 137 | + # _fake_plan_subprocess_run ignores its args and always returns a | ||
| 138 | + # canned success payload -- fine for the "claude" planning call it's | ||
| 139 | + # meant for, but `patch("src.work_graph_plan.subprocess.run", ...)` | ||
| 140 | + # patches the ONE global `subprocess` module every other importer | ||
| 141 | + # shares too (work_graph_plan does `import subprocess`, not | ||
| 142 | + # `from subprocess import run`), so it would ALSO silently hijack | ||
| 143 | + # check_script_compiles's real `py_compile`/`node --check` calls, | ||
| 144 | + # making them always report success regardless of actual content -- | ||
| 145 | + # exactly defeating the failure this test needs to trigger. Only | ||
| 146 | + # intercept the "claude" planning call; let everything else | ||
| 147 | + # (including this test's real compile checks) run for real. | ||
| 148 | + real_subprocess_run = subprocess.run | ||
| 149 | + | ||
| 150 | + def _discriminating_fake(*args, **kwargs): | ||
| 151 | + cmd = args[0] if args else kwargs.get("args") | ||
| 152 | + if cmd and cmd[0] == "claude": | ||
| 153 | + return _fake_plan_subprocess_run(*args, **kwargs) | ||
| 154 | + return real_subprocess_run(*args, **kwargs) | ||
| 155 | + | ||
| 156 | + with patch("src.work_graph_plan.subprocess.run", side_effect=_discriminating_fake), \ | ||
| 157 | + patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 158 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | ||
| 159 | + mock_execute.side_effect = [ | ||
| 160 | + {"content": _FAKE_DOCS["brd"]}, {"content": _FAKE_DOCS["prd"]}, | ||
| 161 | + {"consistent": True, "issues": []}, # PRD consistency check (REQ-02 boundary) | ||
| 162 | + {"content": _FAKE_DOCS["sdd"]}, {"content": _FAKE_DOCS["benchmark"]}, | ||
| 163 | + {"content": _FAKE_DOCS["test_dataset"]}, | ||
| 164 | + {"content": "this is prose, not a real script"}, # test_script: fails compile check | ||
| 165 | + {"content": "still not real code either"}, # the one regeneration attempt: also fails | ||
| 166 | + ] | ||
| 167 | + with self.assertRaises(work_graph_orchestrate.ConsistencyCheckFailedError): | ||
| 168 | + e2e_pipeline.plan_documents(self.conn, "task-1", "goal", self.tmp.name) | ||
| 169 | + | ||
| 170 | + rows = self.conn.execute("SELECT artifact_type FROM artifact WHERE task_id = 'task-1'").fetchall() | ||
| 171 | + persisted = {r[0] for r in rows} | ||
| 172 | + self.assertEqual(persisted, {"brd", "prd", "sdd", "benchmark", "test_dataset"}) | ||
| 173 | + self.assertNotIn("test_script", persisted) | ||
| 174 | + | ||
| 123 | 175 | ||
| 124 | class RunEndToEndTest(PipelineTestBase): | 176 | class RunEndToEndTest(PipelineTestBase): |
| 125 | def test_full_pipeline_wiring(self): | 177 | def test_full_pipeline_wiring(self): |
| @@ -516,9 +568,83 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): | @@ -516,9 +568,83 @@ class ValidateImplementationWithOwnGraphTest(unittest.TestCase): | ||
| 516 | def test_creates_its_own_work_graph_run_row(self): | 568 | def test_creates_its_own_work_graph_run_row(self): |
| 517 | result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") | 569 | result = self._run("import sys\nsys.exit(0)\n", "VERDICT: PASS\nlooks good") |
| 518 | run_row = self.conn.execute( | 570 | run_row = self.conn.execute( |
| 519 | - "SELECT org_node FROM work_graph_run WHERE task_id = 'task-1'" | 571 | + "SELECT org_node, status FROM work_graph_run WHERE task_id = 'task-1'" |
| 520 | ).fetchone() | 572 | ).fetchone() |
| 521 | self.assertEqual(run_row[0], "validator") | 573 | self.assertEqual(run_row[0], "validator") |
| 574 | + # 2026-08-07 (real incident): this used to stay 'running' forever on | ||
| 575 | + # the happy path -- verify_fn returning ok=True means | ||
| 576 | + # _verify_and_maybe_regenerate never touches work_graph_run.status | ||
| 577 | + # at all, so nothing else in this function's old code ever marked | ||
| 578 | + # it terminal. schema.py's `ux_work_graph_run_active` unique index | ||
| 579 | + # covers exactly this non-terminal status set, so a stuck 'running' | ||
| 580 | + # row silently blocked any later re-verify of the SAME task from | ||
| 581 | + # ever creating its own new validator run. | ||
| 582 | + self.assertEqual(run_row[1], "done") | ||
| 583 | + | ||
| 584 | + def test_second_validation_run_does_not_collide_with_the_first(self): | ||
| 585 | + # The actual regression: org_level_loop.run_fix_loop re-verifying | ||
| 586 | + # after a FAIL (or a reconciliation-loop resume retrying) calls this | ||
| 587 | + # function again for the SAME task_id -- must not raise | ||
| 588 | + # "UNIQUE constraint failed: work_graph_run.task_id, | ||
| 589 | + # work_graph_run.org_node" against a first run stuck non-terminal. | ||
| 590 | + self._run("import sys\nsys.exit(1)\n", "VERDICT: FAIL\nreal bug found") | ||
| 591 | + | ||
| 592 | + # Benchmark/dataset content must differ from the first call's -- | ||
| 593 | + # write_artifact does a real `git commit`, which fails with "nothing | ||
| 594 | + # to commit" if a second write produces byte-identical file content | ||
| 595 | + # (the file path isn't run_id-scoped). Real re-verify runs would | ||
| 596 | + # naturally regenerate slightly different content each time; this | ||
| 597 | + # just makes that explicit rather than accidentally relying on it. | ||
| 598 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 599 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute, \ | ||
| 600 | + patch.object(org_node_trigger, "trigger_org_node", return_value="VERDICT: PASS\nfixed now"): | ||
| 601 | + mock_execute.side_effect = [ | ||
| 602 | + {"content": "# Validator's own Benchmark (re-verify)"}, | ||
| 603 | + {"content": "N/A: no dataset needed, still"}, | ||
| 604 | + {"content": "import sys\nsys.exit(0)\n"}, | ||
| 605 | + ] | ||
| 606 | + second = e2e_pipeline.validate_implementation_with_own_graph( | ||
| 607 | + self.conn, "task-1", "sdd content", "prd content", "goal text", self.tmp.name, | ||
| 608 | + ) | ||
| 609 | + | ||
| 610 | + self.assertEqual(second["verdict"], "PASS") | ||
| 611 | + rows = self.conn.execute( | ||
| 612 | + "SELECT status FROM work_graph_run WHERE task_id = 'task-1' ORDER BY rowid" | ||
| 613 | + ).fetchall() | ||
| 614 | + self.assertEqual([r[0] for r in rows], ["done", "done"]) | ||
| 615 | + | ||
| 616 | + def test_earlier_nodes_survive_a_later_node_failing_verification(self): | ||
| 617 | + # 2026-08-07 (same real incident/fix as PlanDocumentsTest's own | ||
| 618 | + # test of this name): validator_test_script fails verify_fn twice | ||
| 619 | + # -- ConsistencyCheckFailedError must still propagate, but | ||
| 620 | + # validator_benchmark/validator_test_dataset (genuinely produced | ||
| 621 | + # and accepted) must be persisted, not discarded along with it. | ||
| 622 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 623 | + patch.object(work_graph_execute, "execute_work_graph") as mock_execute: | ||
| 624 | + mock_execute.side_effect = [ | ||
| 625 | + {"content": "# Validator's own Benchmark"}, | ||
| 626 | + {"content": "N/A: no dataset needed"}, | ||
| 627 | + {"content": "this is prose, not a real script"}, # fails compile check | ||
| 628 | + {"content": "still not real code either"}, # the one regeneration attempt: also fails | ||
| 629 | + ] | ||
| 630 | + with self.assertRaises(work_graph_orchestrate.ConsistencyCheckFailedError): | ||
| 631 | + e2e_pipeline.validate_implementation_with_own_graph( | ||
| 632 | + self.conn, "task-1", "sdd content", "prd content", "goal text", self.tmp.name, | ||
| 633 | + ) | ||
| 634 | + | ||
| 635 | + rows = self.conn.execute("SELECT artifact_type FROM artifact WHERE task_id = 'task-1'").fetchall() | ||
| 636 | + persisted = {r[0] for r in rows} | ||
| 637 | + self.assertEqual(persisted, {"validator_benchmark", "validator_test_dataset"}) | ||
| 638 | + self.assertNotIn("validator_test_script", persisted) | ||
| 639 | + # the work_graph_run itself must NOT be left stuck 'running' either | ||
| 640 | + # (ux_work_graph_run_active would block any later re-verify) -- | ||
| 641 | + # _verify_and_maybe_regenerate's own 'blocked' write on this path | ||
| 642 | + # already covers it; confirming it here catches a regression if | ||
| 643 | + # that ever stops being true. | ||
| 644 | + run_status = self.conn.execute( | ||
| 645 | + "SELECT status FROM work_graph_run WHERE task_id = 'task-1'" | ||
| 646 | + ).fetchone()[0] | ||
| 647 | + self.assertEqual(run_status, "blocked") | ||
| 522 | 648 | ||
| 523 | 649 | ||
| 524 | if __name__ == "__main__": | 650 | if __name__ == "__main__": |
src/work_graph_orchestrate.py
| @@ -82,7 +82,21 @@ def _log_event(conn, node_exec_id, event_type, **payload): | @@ -82,7 +82,21 @@ def _log_event(conn, node_exec_id, event_type, **payload): | ||
| 82 | class ConsistencyCheckFailedError(Exception): | 82 | class ConsistencyCheckFailedError(Exception): |
| 83 | """`verify_fn` rejected a node's output twice in a row (original + | 83 | """`verify_fn` rejected a node's output twice in a row (original + |
| 84 | one regeneration attempt) -- `work_graph_run.status` is already | 84 | one regeneration attempt) -- `work_graph_run.status` is already |
| 85 | - 'blocked' by the time this is raised.""" | 85 | + 'blocked' by the time this is raised. |
| 86 | + | ||
| 87 | + `node_id` (2026-08-07): the rejected node's id, so a caller salvaging | ||
| 88 | + OTHER nodes' already-persisted artifacts after a partial run_graph() | ||
| 89 | + failure (see e2e_pipeline.py's `_write_completed_node_artifacts`) can | ||
| 90 | + tell this one apart from a node that's `work_node_execution.status | ||
| 91 | + ='done'` for a genuinely different reason -- 'done' there only means | ||
| 92 | + "passed schema validation", not "passed verify_fn". The original AND | ||
| 93 | + its "-reflect" retry both end up schema-valid-and-'done' even when | ||
| 94 | + THIS error fires (verify_fn rejected both), so status alone can't | ||
| 95 | + distinguish a verify_fn-rejected node from a genuinely good one.""" | ||
| 96 | + | ||
| 97 | + def __init__(self, message, node_id=None): | ||
| 98 | + super().__init__(message) | ||
| 99 | + self.node_id = node_id | ||
| 86 | 100 | ||
| 87 | 101 | ||
| 88 | class WorkGraphLayerError(Exception): | 102 | class WorkGraphLayerError(Exception): |
| @@ -464,7 +478,8 @@ def _verify_and_maybe_regenerate( | @@ -464,7 +478,8 @@ def _verify_and_maybe_regenerate( | ||
| 464 | conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) | 478 | conn.execute("UPDATE work_graph_run SET status = 'blocked' WHERE run_id = ?", (run_id,)) |
| 465 | conn.commit() | 479 | conn.commit() |
| 466 | raise ConsistencyCheckFailedError( | 480 | raise ConsistencyCheckFailedError( |
| 467 | - "node {} still failed verification after one regeneration attempt: {}".format(node_id, feedback2) | 481 | + "node {} still failed verification after one regeneration attempt: {}".format(node_id, feedback2), |
| 482 | + node_id=node_id, | ||
| 468 | ) | 483 | ) |
| 469 | 484 | ||
| 470 | conn.execute("UPDATE work_graph_run SET status = 'verifying' WHERE run_id = ?", (run_id,)) | 485 | conn.execute("UPDATE work_graph_run SET status = 'verifying' WHERE run_id = ?", (run_id,)) |