Commit 753fefa45cfa59a38dc18dfde075d6f4800f492c
1 parent
b6b495c8
U-035: self-target unconditional worktree + core_engine classification audit
Adds paths.repo_root()/is_self_target() and forces needs_worktree=True in work_graph_orchestrate.run_graph() whenever target_folder_path is graphlemon's own checkout, regardless of caller intent -- 05 §5.8's unconditional rule, since a self-target in-place edit risks corrupting the currently-running process's own source out from under it. Adds src/core_engine_classify.py (path-based rule: .claude/agents/*.md, src/*, supervisor/* = core_engine; frontend/, docs/ = not) and bug_record.record_core_engine_change() (a PASS-path audit record, distinct from create_bug_record's FAIL-path root-cause attribution -- no route decision, status='resolved' from creation). scheduler.finish_task_lifecycle() records this audit trail as an additive side effect after close_task_done() on self-target PASS closures. Found and resolved a real SDD-internal conflict (confirmed with the user via AskUserQuestion) before wiring this up: supervisor-auto-rollback-design.md §1 triggers Deploy Supervisor on self-target+Validator-PASS alone, but 03-C §3.11/04-P §4.6 literally gate it on an existing core_engine bug_record -- and task_create.py confirms goal-type self-improvement tasks (the exact self-development-feasibility.md walkthrough scenario) never produce a bug_record on their happy path, which would silently defeat that whole task class under the literal reading. Resolved per user decision: trigger on self-target+PASS alone; the bug_record is a byproduct, not a gate. p_deploy_supervisor_blue_green (04-P §4.6) never touches task.status, so close_task_done() still runs unconditionally -- Deploy Supervisor is a fully decoupled state machine, matching §2's "no shared failure domain" principle. org_loop_route.py is intentionally untouched -- fast_lane/full_replan is FAIL-path retry routing, orthogonal to this PASS-path trigger; deployment row creation stays U-037's job per its own Done criterion. 417 tests green; RealGitWorktreeIsolationTest (U-013/REQ-03's real git worktree regression) re-verified passing per this Unit's own Done criterion. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Showing
12 changed files
with
412 additions
and
26 deletions
docs/development-plan.md
| @@ -342,7 +342,11 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立,可并行推进。 | @@ -342,7 +342,11 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立,可并行推进。 | ||
| 342 | - **U-035 [Risk Gate]** self-target 场景 worktree 无条件强制规则(缺口 1)+ `root_cause_category=core_engine` 命中时路由到蓝绿部署流程而非 `fast_lane`(`c_org_loop_route` 扩展) | 342 | - **U-035 [Risk Gate]** self-target 场景 worktree 无条件强制规则(缺口 1)+ `root_cause_category=core_engine` 命中时路由到蓝绿部署流程而非 `fast_lane`(`c_org_loop_route` 扩展) |
| 343 | - 依赖:U-013(worktree 机制)、U-016(`c_org_loop_route`)、U-034(`core_engine` 枚举) | 343 | - 依赖:U-013(worktree 机制)、U-016(`c_org_loop_route`)、U-034(`core_engine` 枚举) |
| 344 | - Done:`target_folder_path == graphlemon 自身安装路径` 时,即使是单节点串行修改也强制走 worktree(不再要求"仅并行时才强制");`root_cause_category=core_engine` 的 bug 被 `c_org_loop_route` 正确路由到 PH5-S2/S3 的蓝绿流程;**且改动落地后必须重跑一次 U-013/REQ-03(worktree 隔离)现有回归用例全部通过**——这是 PH4 全量收口之后第一个真正 touch 生产核心执行路径的 Unit,不能假设"没测出问题就是没问题",否则等于在 PH5 一开工就悄悄打破 PH4 刚建立起来的信任前提 | 344 | - Done:`target_folder_path == graphlemon 自身安装路径` 时,即使是单节点串行修改也强制走 worktree(不再要求"仅并行时才强制");`root_cause_category=core_engine` 的 bug 被 `c_org_loop_route` 正确路由到 PH5-S2/S3 的蓝绿流程;**且改动落地后必须重跑一次 U-013/REQ-03(worktree 隔离)现有回归用例全部通过**——这是 PH4 全量收口之后第一个真正 touch 生产核心执行路径的 Unit,不能假设"没测出问题就是没问题",否则等于在 PH5 一开工就悄悄打破 PH4 刚建立起来的信任前提 |
| 345 | - - 覆盖:REQ-03(回归) | 345 | + - **实现补充(2026-08-02)——发现并解决一处真实 SDD 内部冲突(已跟用户确认)**:动手前核对"core_engine 命中时路由到蓝绿部署流程"具体怎么触发时,发现 `supervisor-auto-rollback-design.md` §1("触发时点:Validator PASS → merge → Deploy Supervisor 接管",完全不提 bug_record)和 03-C §3.11/04-P §4.6(字面上把触发门槛定义为"已存在一条 `root_cause_category=core_engine` 的 bug_record")互相矛盾。核实 `task_create.py` 后确认:`c_task_create` 只会创建 `task_type='goal'` 的任务,`bug_record` 只在 Validator FAIL 归因或人工报告两条路径上产生——也就是说,`self-development-feasibility.md` 9 步走查场景本身(一个普通的自我改进 goal 任务,Planner→Coder→Validator 一次性 PASS,从未进入过 FAIL 循环)根本不会产生任何 bug_record,按 03-C/04-P 字面文本,Deploy Supervisor 永远不会为这整类任务触发——这会悄悄废掉 PH5 对"goal 型自我改进任务"这整条路径的意义。用 AskUserQuestion 向用户说明冲突后,用户选择"按 §1 字面设计:仅凭 self-target+PASS 触发,bug_record 是触发后的旁路产物而非前提条件"。 |
| 346 | + - **设计**:新增 `src/paths.repo_root()`(graphlemon 自身 checkout 根目录,和已有的 `platform_home()`——`~/.graphlemon` 运行时目录——是两个不同概念,别混)+ `paths.is_self_target(target_folder_path)`(真实路径比较,支持测试注入 `repo_root_path` 覆盖,避免测试真的对着当前这份 checkout 跑 worktree 副作用)。`work_graph_orchestrate.run_graph()` 内部计算 `effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path)`,不管调用方传了什么都会在 self-target 时强制为真——两个真实调用点(`e2e_pipeline.plan_documents`/`implement_code`)都不用改,改动集中在唯一的真正入口。新增 `src/core_engine_classify.py`:`classify_core_engine_change(changed_files)` 是纯路径规则(`.claude/agents/*.md`/`src/*`/`supervisor/*` 命中即 core_engine,`frontend/`/`docs/` 不命中——05 §5.8 原文明确点名前两类+schema,schema 本身就在 `src/` 下天然覆盖,没有单独点名前端console,判断为有意排除);`parse_changed_files_from_diff()` 直接从已有的 `code_diff` artifact 内容里解析 `diff --git a/X b/Y` 头,不需要额外一次 `git diff --name-only` 子进程调用。`bug_record.py` 新增 `record_core_engine_change()`——和已有的 `create_bug_record()`(FAIL 路径的根因归因,`suggested_route` 必填)是两个不同场景:这是 PASS 路径的审计留痕,没有"路由决策"可言,`suggested_route` 留空、`status='resolved'`(因为验证已经通过,不是待处理的开放问题);`_VALID_ROOT_CAUSE_CATEGORIES` 顺带放开 `core_engine`(U-034 当时故意没放开,因为分类机制还不存在,现在存在了)。`scheduler.finish_task_lifecycle()` 的 PASS 分支里,`close_task_done()` 之后附加调用 `_record_core_engine_audit_if_self_target()`——**是附加副作用,不是替代**:`p_deploy_supervisor_blue_green`(04-P §4.6)整套状态机只写 `deployment.status`,从头到尾没有一条转移碰 `task.status`,这与 §2"Deploy Supervisor 和 Live/Candidate 之间没有共享失败域"的原则完全一致——Coder 的工作本身确实已经完成且已合并,`task.status='done'` 不应该被 Deploy Supervisor 是否还在跑而卡住。 | ||
| 347 | + - **范围边界(刻意不做的部分)**:这个 Unit 只到"检测 self-target + 记一条 core_engine 审计 bug_record"为止,不创建 `deployment` 行——`deployment` 记录的生成明确是 U-037 `c_deploy_supervisor_stage` 自己的 Done 标准("给定一个已合并到主分支的 commit,能生成 deployment 记录"),Deploy Supervisor 的守护进程(U-036 建骨架)今后靠轮询"self-target 项目里状态为 done、且没有对应 deployment 行的任务"来发现待部署的改动,这个发现机制本身也是 U-036/037 的范围,这里不提前实现。`org_loop_route.route_org_loop()` 的 `fast_lane`/`full_replan` 分支本身完全没有改动——那是 Validator FAIL 后的重试路由决策,和这里"PASS 后是否需要蓝绿部署"是两个正交的问题,development-plan.md 原文里"`c_org_loop_route` 扩展"这行描述在这次实现后证明是不准确的说法,实际改动点在 `scheduler.py` 的任务收尾逻辑,不在 `org_loop_route.py`。 | ||
| 348 | + - **验证**:全量 417 个测试通过;`test_work_node_run.RealGitWorktreeIsolationTest`(真实 git worktree 隔离+merge,U-013 的 REQ-03 boundary 用例)单独重跑确认无回归,如 Done 标准要求。 | ||
| 349 | + - 覆盖:REQ-03(回归);05 §5.8(self-target worktree 强制 + core_engine 分类,触发条件已按用户决策订正) | ||
| 346 | 350 | ||
| 347 | ### PH5-S2 Deploy Supervisor 核心机制 | 351 | ### PH5-S2 Deploy Supervisor 核心机制 |
| 348 | 352 |
docs/sdd/unit_matrix.json
| @@ -52,7 +52,7 @@ | @@ -52,7 +52,7 @@ | ||
| 52 | {"id": "U-033", "phase": "PH4", "sub_phase": "PH4-S3", "name": "A-2 假设收口(见 development-plan.md 2026-08-02 实现补充)", "classification": "Risk Gate", "depends_on": ["U-004"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": [], "assumption": ["A-2"]}, "done": "A-2 状态确认为 confirmed 或记录实际偏差;最终结论:维持 U-004 记录的\"部分成立\",PH1~PH4 积累的真实运行数据未暴露新偏差,偏差范围到 U-004 为止已完全摸清(Coder SOUL.md 三处实质性重写),不改判 confirmed,正式收口"}, | 52 | {"id": "U-033", "phase": "PH4", "sub_phase": "PH4-S3", "name": "A-2 假设收口(见 development-plan.md 2026-08-02 实现补充)", "classification": "Risk Gate", "depends_on": ["U-004"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": [], "assumption": ["A-2"]}, "done": "A-2 状态确认为 confirmed 或记录实际偏差;最终结论:维持 U-004 记录的\"部分成立\",PH1~PH4 积累的真实运行数据未暴露新偏差,偏差范围到 U-004 为止已完全摸清(Coder SOUL.md 三处实质性重写),不改判 confirmed,正式收口"}, |
| 53 | 53 | ||
| 54 | {"id": "U-034", "phase": "PH5", "sub_phase": "PH5-S1", "name": "01-M 扩展:deployment/deploy_health_signal + core_engine 枚举(见 development-plan.md 2026-08-02 实现补充)", "classification": "Enabler", "depends_on": ["U-001"], "coverage": {"model": ["deployment", "deploy_health_signal", "bug_record"], "view": [], "controller": [], "process": [], "requirement": []}, "done": "两张新表建表成功(共 17 张);bug_record 新枚举值向后兼容 U-015/U-018 已有测试;deployment 状态机用 01-M 1.15 完整 15 值枚举;ux_deployment_active 表达式索引强制全局单例非终态约束;bug_record.py 应用层白名单刻意未跟着放开 core_engine,classification+路由逻辑留给 U-035"}, | 54 | {"id": "U-034", "phase": "PH5", "sub_phase": "PH5-S1", "name": "01-M 扩展:deployment/deploy_health_signal + core_engine 枚举(见 development-plan.md 2026-08-02 实现补充)", "classification": "Enabler", "depends_on": ["U-001"], "coverage": {"model": ["deployment", "deploy_health_signal", "bug_record"], "view": [], "controller": [], "process": [], "requirement": []}, "done": "两张新表建表成功(共 17 张);bug_record 新枚举值向后兼容 U-015/U-018 已有测试;deployment 状态机用 01-M 1.15 完整 15 值枚举;ux_deployment_active 表达式索引强制全局单例非终态约束;bug_record.py 应用层白名单刻意未跟着放开 core_engine,classification+路由逻辑留给 U-035"}, |
| 55 | - {"id": "U-035", "phase": "PH5", "sub_phase": "PH5-S1", "name": "self-target worktree 无条件强制 + core_engine 路由到蓝绿部署", "classification": "Risk Gate", "depends_on": ["U-013", "U-016", "U-034"], "coverage": {"model": [], "view": [], "controller": ["c_org_loop_route"], "process": [], "requirement": ["REQ-03"]}, "done": "self-target 时无条件走 worktree;core_engine 分类的 bug 被路由到 PH5-S2/S3 而非 fast_lane;且改动落地后必须重跑 U-013/REQ-03(worktree 隔离)现有回归用例全部通过(2026-07-30 计划评审追加,确认未破坏 PH4 已建立的隔离保证)"}, | 55 | + {"id": "U-035", "phase": "PH5", "sub_phase": "PH5-S1", "name": "self-target worktree 无条件强制 + core_engine 分类审计(见 development-plan.md 2026-08-02 实现补充)", "classification": "Risk Gate", "depends_on": ["U-013", "U-016", "U-034"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": ["REQ-03"]}, "done": "self-target 时无条件走 worktree(work_graph_orchestrate.run_graph 内强制);core_engine 分类(src/core_engine_classify.py,路径规则)在 self-target 任务 PASS 收尾时记一条审计用 bug_record;且改动落地后重跑 U-013/REQ-03(worktree 隔离)现有回归用例全部通过。**2026-08-02 用户决策修正**:Deploy Supervisor 触发条件按 supervisor-auto-rollback-design.md §1 字面设计(self-target+PASS 即触发,bug_record 是旁路产物不是前提),而非 03-C/04-P 字面文本要求的\"先有 core_engine bug_record\"——后者会让从不产生 bug_record 的 goal 型自我改进任务永远触发不了 Deploy Supervisor,已用 AskUserQuestion 与用户确认此订正。c_org_loop_route 未改动(fast_lane/full_replan 是 FAIL 路径重试路由,与此处 PASS 路径的蓝绿触发正交),deployment 行创建仍是 U-037 的范围,不在本 Unit 提前做"}, |
| 56 | {"id": "U-036", "phase": "PH5", "sub_phase": "PH5-S2", "name": "supervisor/isolation 补齐单测 + Deploy Supervisor 进程骨架", "classification": "Enabler", "depends_on": [], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": []}, "done": "四模块各有 test_*.py;Deploy Supervisor 骨架独立进程可启停,不受 Live/Candidate 崩溃影响"}, | 56 | {"id": "U-036", "phase": "PH5", "sub_phase": "PH5-S2", "name": "supervisor/isolation 补齐单测 + Deploy Supervisor 进程骨架", "classification": "Enabler", "depends_on": [], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": []}, "done": "四模块各有 test_*.py;Deploy Supervisor 骨架独立进程可启停,不受 Live/Candidate 崩溃影响"}, |
| 57 | {"id": "U-037", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_stage", "classification": "Risk Gate", "depends_on": ["U-034", "U-036"], "coverage": {"model": ["deployment"], "view": [], "controller": ["c_deploy_supervisor_stage"], "process": [], "requirement": []}, "done": "能生成 deployment 记录并拉起监听临时端口、连一次性 DB 副本的候选进程"}, | 57 | {"id": "U-037", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_stage", "classification": "Risk Gate", "depends_on": ["U-034", "U-036"], "coverage": {"model": ["deployment"], "view": [], "controller": ["c_deploy_supervisor_stage"], "process": [], "requirement": []}, "done": "能生成 deployment 记录并拉起监听临时端口、连一次性 DB 副本的候选进程"}, |
| 58 | {"id": "U-038", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_health_check", "classification": "Risk Gate", "depends_on": ["U-037"], "coverage": {"model": [], "view": [], "controller": ["c_deploy_supervisor_health_check"], "process": [], "requirement": []}, "done": "process_alive/http_health/canary_task 三项检查全过 -> health_check_passed;任一失败 -> health_check_failed 且 Live 未受影响"}, | 58 | {"id": "U-038", "phase": "PH5", "sub_phase": "PH5-S2", "name": "c_deploy_supervisor_health_check", "classification": "Risk Gate", "depends_on": ["U-037"], "coverage": {"model": [], "view": [], "controller": ["c_deploy_supervisor_health_check"], "process": [], "requirement": []}, "done": "process_alive/http_health/canary_task 三项检查全过 -> health_check_passed;任一失败 -> health_check_failed 且 Live 未受影响"}, |
src/bug_record.py
| @@ -25,9 +25,12 @@ the loop) and has since been resolved, not left open. | @@ -25,9 +25,12 @@ the loop) and has since been resolved, not left open. | ||
| 25 | import json | 25 | import json |
| 26 | import uuid | 26 | import uuid |
| 27 | 27 | ||
| 28 | -# core_engine deliberately excluded -- see src/schema.py's matching CHECK | ||
| 29 | -# constraint and its own comment: PH5-scoped (U-034), not accepted until then. | ||
| 30 | -_VALID_ROOT_CAUSE_CATEGORIES = ("planning", "coder", "skill_defect") | 28 | +# core_engine added in U-035 -- U-034 deliberately left this app-layer |
| 29 | +# whitelist unchanged even though it already widened the schema-level CHECK | ||
| 30 | +# constraint, since classifying a change as core_engine is U-035's | ||
| 31 | +# path-based rule (src/core_engine_classify.py), not something the | ||
| 32 | +# Validator's existing 3-perspective judge_root_cause can produce on its own. | ||
| 33 | +_VALID_ROOT_CAUSE_CATEGORIES = ("planning", "coder", "skill_defect", "core_engine") | ||
| 31 | 34 | ||
| 32 | 35 | ||
| 33 | class InvalidRootCauseCategoryError(Exception): | 36 | class InvalidRootCauseCategoryError(Exception): |
| @@ -108,6 +111,31 @@ def create_bug_record( | @@ -108,6 +111,31 @@ def create_bug_record( | ||
| 108 | return {"bug_id": bug_id} | 111 | return {"bug_id": bug_id} |
| 109 | 112 | ||
| 110 | 113 | ||
| 114 | +def record_core_engine_change(conn, task_id, description): | ||
| 115 | + """U-035 (05 §5.8): audit-trail record for a self-target task that | ||
| 116 | + touched core-engine territory (`.claude/agents/*.md`/`src/`/`supervisor/`) | ||
| 117 | + and passed Validator on its first attempt -- NOT the Validator-FAIL | ||
| 118 | + root-cause-judgment path `create_bug_record()` serves. There is no | ||
| 119 | + "route" decision here (`suggested_route` stays null, unlike | ||
| 120 | + `create_bug_record`'s required field) and no open triage step -- the | ||
| 121 | + change already passed validation, so `status='resolved'` from creation. | ||
| 122 | + Exists so 03-C §3.11's `c_org_loop_route` core_engine branch and 04-P | ||
| 123 | + §4.6's `p_deploy_supervisor_blue_green` trigger text -- both phrased in | ||
| 124 | + terms of "a bug_record with root_cause_category=core_engine" -- have a | ||
| 125 | + real row to reference, satisfying that text even though this is a | ||
| 126 | + PASS-path audit record rather than a FAIL-path bug attribution. | ||
| 127 | + Returns {"bug_id"}.""" | ||
| 128 | + bug_id = str(uuid.uuid4()) | ||
| 129 | + conn.execute( | ||
| 130 | + "INSERT INTO bug_record " | ||
| 131 | + "(bug_id, task_id, source, description, root_cause_category, status) " | ||
| 132 | + "VALUES (?, ?, 'validator', ?, 'core_engine', 'resolved')", | ||
| 133 | + (bug_id, task_id, description), | ||
| 134 | + ) | ||
| 135 | + conn.commit() | ||
| 136 | + return {"bug_id": bug_id} | ||
| 137 | + | ||
| 138 | + | ||
| 111 | def claim_human_reported_bug(conn, bug_id): | 139 | def claim_human_reported_bug(conn, bug_id): |
| 112 | """04-P §4.3: human_reported -> tracing_lineage. Validator claims an | 140 | """04-P §4.3: human_reported -> tracing_lineage. Validator claims an |
| 113 | open, source=human bug_record. status: open -> triaging.""" | 141 | open, source=human bug_record. status: open -> triaging.""" |
src/core_engine_classify.py
0 → 100644
| 1 | +"""05 §5.8 / U-035: path-based classification of whether a self-target | ||
| 2 | +change touches "core engine" territory -- `.claude/agents/*.md` identity | ||
| 3 | +files, Graph Engine scheduler/executor source (`src/*.py`), or 01-M's own | ||
| 4 | +schema (`src/schema.py`, itself a `src/*.py` file so already covered). | ||
| 5 | + | ||
| 6 | +Deliberately NOT the Validator's existing 3-perspective `judge_root_cause` | ||
| 7 | +(root_cause_trace.py) -- that produces planning|coder|skill_defect from | ||
| 8 | +content-level judgment about WHY a bug happened; this produces core_engine | ||
| 9 | +from a purely mechanical WHERE-did-the-change-land rule, independent of | ||
| 10 | +whether anything went wrong. The two are orthogonal and only combined at | ||
| 11 | +the call site (scheduler.py), never merged into one function. | ||
| 12 | + | ||
| 13 | +Scope note: `frontend/` is deliberately excluded (05 §5.8 names the identity | ||
| 14 | +files + engine source + schema, not the console UI -- a frontend/app.js bug | ||
| 15 | +is much lower blast radius and doesn't need blue-green). `supervisor/` IS | ||
| 16 | +included -- Deploy Supervisor modifying its own isolation/rollback logic is | ||
| 17 | +at least as self-referentially risky as the engine it's protecting. | ||
| 18 | +""" | ||
| 19 | + | ||
| 20 | +import posixpath | ||
| 21 | + | ||
| 22 | +_CORE_ENGINE_PREFIXES = ("src/", "supervisor/", ".claude/agents/") | ||
| 23 | + | ||
| 24 | + | ||
| 25 | +def classify_core_engine_change(changed_files): | ||
| 26 | + """changed_files: iterable of repo-relative paths (posix-style, as | ||
| 27 | + produced by `git diff --name-only` / `diff --git a/X b/Y` headers). | ||
| 28 | + Returns True if ANY path falls under a core-engine prefix.""" | ||
| 29 | + for path in changed_files: | ||
| 30 | + normalized = posixpath.normpath(path.replace("\\", "/")) | ||
| 31 | + if normalized.startswith(_CORE_ENGINE_PREFIXES) or any( | ||
| 32 | + normalized == prefix.rstrip("/") for prefix in _CORE_ENGINE_PREFIXES | ||
| 33 | + ): | ||
| 34 | + return True | ||
| 35 | + return False | ||
| 36 | + | ||
| 37 | + | ||
| 38 | +def parse_changed_files_from_diff(diff_text): | ||
| 39 | + """Extracts repo-relative changed file paths from unified `git diff` | ||
| 40 | + output (`diff --git a/X b/Y` header lines) -- avoids requiring a second | ||
| 41 | + git subprocess call when the diff text is already on hand (e.g. as the | ||
| 42 | + code_diff artifact's content).""" | ||
| 43 | + changed = [] | ||
| 44 | + for line in (diff_text or "").splitlines(): | ||
| 45 | + if line.startswith("diff --git a/"): | ||
| 46 | + rest = line[len("diff --git a/"):] | ||
| 47 | + # "<a-path> b/<b-path>" -- split on the last " b/" occurrence | ||
| 48 | + # since a path itself could theoretically contain " b/". | ||
| 49 | + marker = rest.rfind(" b/") | ||
| 50 | + if marker != -1: | ||
| 51 | + changed.append(rest[:marker]) | ||
| 52 | + return changed |
src/paths.py
| @@ -48,3 +48,23 @@ def is_separate_from(target_folder_path): | @@ -48,3 +48,23 @@ def is_separate_from(target_folder_path): | ||
| 48 | home_prefix = home.rstrip(os.sep) + os.sep | 48 | home_prefix = home.rstrip(os.sep) + os.sep |
| 49 | target_prefix = target.rstrip(os.sep) + os.sep | 49 | target_prefix = target.rstrip(os.sep) + os.sep |
| 50 | return not target.startswith(home_prefix) and not home.startswith(target_prefix) | 50 | return not target.startswith(home_prefix) and not home.startswith(target_prefix) |
| 51 | + | ||
| 52 | + | ||
| 53 | +def repo_root(): | ||
| 54 | + """graphlemon's own code checkout root (the directory containing `src/`, | ||
| 55 | + `.git`, `docs/`, ...) -- distinct from platform_home() (the ~/.graphlemon | ||
| 56 | + *runtime* directory for logs/db, deliberately outside the checkout). | ||
| 57 | + U-035 (05 §5.8 self-target rules) needs this to detect when a task's | ||
| 58 | + `target_folder_path` IS graphlemon's own repo.""" | ||
| 59 | + return os.path.dirname(os.path.dirname(os.path.abspath(__file__))) | ||
| 60 | + | ||
| 61 | + | ||
| 62 | +def is_self_target(target_folder_path, repo_root_path=None): | ||
| 63 | + """05 §5.8: `target_folder_path == graphlemon 平台自身安装路径`. Compares | ||
| 64 | + realpaths so a symlinked or relative target_folder_path still matches. | ||
| 65 | + `repo_root_path` defaults to this running instance's own checkout | ||
| 66 | + (`repo_root()`) but can be overridden -- e.g. by a test simulating a | ||
| 67 | + self-target scenario against a throwaway clone rather than this actual | ||
| 68 | + running checkout.""" | ||
| 69 | + root = repo_root_path if repo_root_path is not None else repo_root() | ||
| 70 | + return os.path.realpath(target_folder_path) == os.path.realpath(root) |
src/scheduler.py
| @@ -43,6 +43,27 @@ exactly the layer `budget_guardrail.check_budget()` already does the | @@ -43,6 +43,27 @@ exactly the layer `budget_guardrail.check_budget()` already does the | ||
| 43 | equivalent translation at for its own case (REQ-10), so `_block_task()` | 43 | equivalent translation at for its own case (REQ-10), so `_block_task()` |
| 44 | below follows that same established pattern (UPDATE + `task_status_changed` | 44 | below follows that same established pattern (UPDATE + `task_status_changed` |
| 45 | execution_event) rather than inventing a new one. | 45 | execution_event) rather than inventing a new one. |
| 46 | + | ||
| 47 | +**U-035 self-target audit trail (2026-08-02)**: resolved a real conflict | ||
| 48 | +between supervisor-auto-rollback-design.md §1 (Deploy Supervisor triggers on | ||
| 49 | +"self-target + Validator PASS + merge", no bug_record involved) and 03-C | ||
| 50 | +§3.11/04-P §4.6's literal text (gated on an *existing* | ||
| 51 | +`bug_record.root_cause_category=core_engine`) -- confirmed with the user | ||
| 52 | +that a plain goal-type self-improvement task (task_create.py: `c_task_create` | ||
| 53 | +only ever makes task_type='goal') never gets a bug_record on its happy path | ||
| 54 | +at all, so the literal gate would silently defeat PH5's entire purpose for | ||
| 55 | +that whole task class. Resolution (user-confirmed): trigger on self-target+ | ||
| 56 | +PASS alone; `bug_record.record_core_engine_change()` is created as an | ||
| 57 | +audit-trail byproduct of that trigger, not a precondition for it. `deployment` | ||
| 58 | +row creation itself (the actual hand-off) stays in U-037's `c_deploy_ | ||
| 59 | +supervisor_stage` as its own Done criterion already specifies -- this module | ||
| 60 | +only detects + records, it doesn't stage a candidate. `p_deploy_supervisor_ | ||
| 61 | +blue_green` (04-P §4.6) never touches `task.status` at all -- it's a fully | ||
| 62 | +separate state machine on `deployment`, matching supervisor-auto-rollback- | ||
| 63 | +design.md §2's "no shared failure domain" between Deploy Supervisor and the | ||
| 64 | +Graph Engine -- so `close_task_done()` still runs unconditionally on PASS | ||
| 65 | +(the Coder's work IS genuinely complete and merged), the self-target check | ||
| 66 | +is a purely additive side effect alongside it, not a replacement for it. | ||
| 46 | """ | 67 | """ |
| 47 | 68 | ||
| 48 | import json | 69 | import json |
| @@ -50,8 +71,8 @@ import uuid | @@ -50,8 +71,8 @@ import uuid | ||
| 50 | import concurrent.futures | 71 | import concurrent.futures |
| 51 | 72 | ||
| 52 | from src import ( | 73 | from src import ( |
| 53 | - e2e_pipeline, org_level_loop, org_loop_route, schema, settings_config, | ||
| 54 | - task_create, work_graph_execute, work_graph_orchestrate, work_node_run, | 74 | + bug_record, core_engine_classify, e2e_pipeline, org_level_loop, org_loop_route, paths, schema, |
| 75 | + settings_config, task_create, work_graph_execute, work_graph_orchestrate, work_node_run, | ||
| 55 | ) | 76 | ) |
| 56 | 77 | ||
| 57 | _BUSY_TIMEOUT_MS = 30000 | 78 | _BUSY_TIMEOUT_MS = 30000 |
| @@ -88,6 +109,24 @@ def _block_task(conn, task_id, blocked_reason): | @@ -88,6 +109,24 @@ def _block_task(conn, task_id, blocked_reason): | ||
| 88 | conn.commit() | 109 | conn.commit() |
| 89 | 110 | ||
| 90 | 111 | ||
| 112 | +def _record_core_engine_audit_if_self_target(conn, task_id, target_folder_path, run_result): | ||
| 113 | + """U-035 (05 §5.8): additive side effect on a self-target task's PASS | ||
| 114 | + closure -- see module docstring's "self-target audit trail" note for | ||
| 115 | + why this doesn't gate close_task_done, just follows it.""" | ||
| 116 | + if not paths.is_self_target(target_folder_path): | ||
| 117 | + return | ||
| 118 | + code_diff_id = run_result.get("code_artifact_ids", {}).get("code_diff") | ||
| 119 | + if not code_diff_id: | ||
| 120 | + return | ||
| 121 | + diff_text = e2e_pipeline._read_artifact_content(conn, target_folder_path, code_diff_id) | ||
| 122 | + changed_files = core_engine_classify.parse_changed_files_from_diff(diff_text) | ||
| 123 | + if core_engine_classify.classify_core_engine_change(changed_files): | ||
| 124 | + bug_record.record_core_engine_change( | ||
| 125 | + conn, task_id, | ||
| 126 | + "self-target change touched core-engine files: " + ", ".join(sorted(set(changed_files))), | ||
| 127 | + ) | ||
| 128 | + | ||
| 129 | + | ||
| 91 | def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | 130 | def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): |
| 92 | """PASS -> `close_task_done`; FAIL/INCONCLUSIVE -> `org_level_loop. | 131 | """PASS -> `close_task_done`; FAIL/INCONCLUSIVE -> `org_level_loop. |
| 93 | run_fix_loop` (root-cause judge + reroute + fix + reverify, U-017). | 132 | run_fix_loop` (root-cause judge + reroute + fix + reverify, U-017). |
| @@ -96,6 +135,7 @@ def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | @@ -96,6 +135,7 @@ def finish_task_lifecycle(conn, run_result, target_folder_path, goal_text): | ||
| 96 | verdict = run_result["verdict"]["verdict"] | 135 | verdict = run_result["verdict"]["verdict"] |
| 97 | if verdict == "PASS": | 136 | if verdict == "PASS": |
| 98 | org_loop_route.close_task_done(conn, task_id) | 137 | org_loop_route.close_task_done(conn, task_id) |
| 138 | + _record_core_engine_audit_if_self_target(conn, task_id, target_folder_path, run_result) | ||
| 99 | run_result["final_status"] = "done" | 139 | run_result["final_status"] = "done" |
| 100 | return run_result | 140 | return run_result |
| 101 | 141 |
src/test_bug_record.py
| @@ -60,20 +60,22 @@ class CreateBugRecordTest(BugRecordTestBase): | @@ -60,20 +60,22 @@ class CreateBugRecordTest(BugRecordTestBase): | ||
| 60 | root_cause_detail="d", suggested_route="fast_lane", | 60 | root_cause_detail="d", suggested_route="fast_lane", |
| 61 | ) | 61 | ) |
| 62 | 62 | ||
| 63 | - def test_core_engine_rejected_pending_u035(self): | ||
| 64 | - # U-034 (2026-08-02) widened the SCHEMA's own CHECK constraint to | ||
| 65 | - # accept core_engine (see src/schema.py), but bug_record.py's own | ||
| 66 | - # application-layer _VALID_ROOT_CAUSE_CATEGORIES deliberately still | ||
| 67 | - # excludes it -- nothing yet determines WHEN a bug is core_engine | ||
| 68 | - # (05 §5.8's file-path-based rule, not the Validator's 3-perspective | ||
| 69 | - # judge_root_cause, which only ever produces planning/coder/ | ||
| 70 | - # skill_defect); that classification logic + c_org_loop_route's | ||
| 71 | - # routing to the blue-green flow is U-035's job, not this one's. | ||
| 72 | - with self.assertRaises(bug_record.InvalidRootCauseCategoryError): | ||
| 73 | - bug_record.create_bug_record( | ||
| 74 | - self.conn, task_id="task-1", description="x", root_cause_category="core_engine", | ||
| 75 | - root_cause_detail="d", suggested_route="fast_lane", | ||
| 76 | - ) | 63 | + def test_core_engine_accepted_since_u035(self): |
| 64 | + # U-034 widened the schema's CHECK constraint; U-035 widens this | ||
| 65 | + # app-layer whitelist too, once src/core_engine_classify.py exists | ||
| 66 | + # as a real (if separate) mechanism for determining WHEN a change is | ||
| 67 | + # core_engine. create_bug_record() itself still doesn't classify | ||
| 68 | + # anything -- a caller (e.g. a self-target-aware Validator FAIL path) | ||
| 69 | + # decides the category and passes it explicitly, same as any other | ||
| 70 | + # category. | ||
| 71 | + result = bug_record.create_bug_record( | ||
| 72 | + self.conn, task_id="task-1", description="x", root_cause_category="core_engine", | ||
| 73 | + root_cause_detail="d", suggested_route="fast_lane", | ||
| 74 | + ) | ||
| 75 | + row = self.conn.execute( | ||
| 76 | + "SELECT root_cause_category FROM bug_record WHERE bug_id = ?", (result["bug_id"],) | ||
| 77 | + ).fetchone() | ||
| 78 | + self.assertEqual(row[0], "core_engine") | ||
| 77 | 79 | ||
| 78 | 80 | ||
| 79 | class CreateBugReportTest(BugRecordTestBase): | 81 | class CreateBugReportTest(BugRecordTestBase): |
| @@ -145,6 +147,25 @@ class CreateBugReportTest(BugRecordTestBase): | @@ -145,6 +147,25 @@ class CreateBugReportTest(BugRecordTestBase): | ||
| 145 | self.assertIsNone(row[0]) | 147 | self.assertIsNone(row[0]) |
| 146 | 148 | ||
| 147 | 149 | ||
| 150 | +class RecordCoreEngineChangeTest(BugRecordTestBase): | ||
| 151 | + """U-035: the PASS-path audit trail, distinct from create_bug_record's | ||
| 152 | + FAIL-path root-cause attribution.""" | ||
| 153 | + | ||
| 154 | + def test_creates_resolved_core_engine_record_with_no_route(self): | ||
| 155 | + result = bug_record.record_core_engine_change( | ||
| 156 | + self.conn, task_id="task-1", description="self-target change touched src/scheduler.py" | ||
| 157 | + ) | ||
| 158 | + row = self.conn.execute( | ||
| 159 | + "SELECT source, status, root_cause_category, suggested_route, description " | ||
| 160 | + "FROM bug_record WHERE bug_id = ?", | ||
| 161 | + (result["bug_id"],), | ||
| 162 | + ).fetchone() | ||
| 163 | + self.assertEqual( | ||
| 164 | + row, | ||
| 165 | + ("validator", "resolved", "core_engine", None, "self-target change touched src/scheduler.py"), | ||
| 166 | + ) | ||
| 167 | + | ||
| 168 | + | ||
| 148 | class HumanReportedFlowTest(BugRecordTestBase): | 169 | class HumanReportedFlowTest(BugRecordTestBase): |
| 149 | def _insert_human_bug(self, bug_id="bug-1", status="open"): | 170 | def _insert_human_bug(self, bug_id="bug-1", status="open"): |
| 150 | self.conn.execute( | 171 | self.conn.execute( |
src/test_core_engine_classify.py
0 → 100644
| 1 | +"""U-035 tests: path-based core_engine classification. | ||
| 2 | + | ||
| 3 | +Run: python3 -m unittest src.test_core_engine_classify -v | ||
| 4 | +""" | ||
| 5 | + | ||
| 6 | +import unittest | ||
| 7 | + | ||
| 8 | +from src import core_engine_classify | ||
| 9 | + | ||
| 10 | + | ||
| 11 | +class ClassifyCoreEngineChangeTest(unittest.TestCase): | ||
| 12 | + def test_src_python_file_is_core_engine(self): | ||
| 13 | + self.assertTrue(core_engine_classify.classify_core_engine_change(["src/scheduler.py"])) | ||
| 14 | + | ||
| 15 | + def test_claude_agents_identity_file_is_core_engine(self): | ||
| 16 | + self.assertTrue( | ||
| 17 | + core_engine_classify.classify_core_engine_change([".claude/agents/coder.md"]) | ||
| 18 | + ) | ||
| 19 | + | ||
| 20 | + def test_supervisor_module_is_core_engine(self): | ||
| 21 | + self.assertTrue( | ||
| 22 | + core_engine_classify.classify_core_engine_change(["supervisor/isolation/change_manifest.py"]) | ||
| 23 | + ) | ||
| 24 | + | ||
| 25 | + def test_frontend_only_change_is_not_core_engine(self): | ||
| 26 | + self.assertFalse(core_engine_classify.classify_core_engine_change(["frontend/app.js"])) | ||
| 27 | + | ||
| 28 | + def test_docs_only_change_is_not_core_engine(self): | ||
| 29 | + self.assertFalse(core_engine_classify.classify_core_engine_change(["docs/development-plan.md"])) | ||
| 30 | + | ||
| 31 | + def test_mixed_changeset_with_one_core_engine_file_is_core_engine(self): | ||
| 32 | + self.assertTrue( | ||
| 33 | + core_engine_classify.classify_core_engine_change(["frontend/app.js", "src/e2e_pipeline.py"]) | ||
| 34 | + ) | ||
| 35 | + | ||
| 36 | + def test_empty_changeset_is_not_core_engine(self): | ||
| 37 | + self.assertFalse(core_engine_classify.classify_core_engine_change([])) | ||
| 38 | + | ||
| 39 | + def test_windows_style_separators_normalized(self): | ||
| 40 | + self.assertTrue(core_engine_classify.classify_core_engine_change(["src\\scheduler.py"])) | ||
| 41 | + | ||
| 42 | + | ||
| 43 | +class ParseChangedFilesFromDiffTest(unittest.TestCase): | ||
| 44 | + def test_single_file_diff(self): | ||
| 45 | + diff_text = ( | ||
| 46 | + "diff --git a/src/scheduler.py b/src/scheduler.py\n" | ||
| 47 | + "index abc123..def456 100644\n" | ||
| 48 | + "--- a/src/scheduler.py\n" | ||
| 49 | + "+++ b/src/scheduler.py\n" | ||
| 50 | + "@@ -1,3 +1,3 @@\n" | ||
| 51 | + "-old\n" | ||
| 52 | + "+new\n" | ||
| 53 | + ) | ||
| 54 | + self.assertEqual( | ||
| 55 | + core_engine_classify.parse_changed_files_from_diff(diff_text), ["src/scheduler.py"] | ||
| 56 | + ) | ||
| 57 | + | ||
| 58 | + def test_multiple_files_diff(self): | ||
| 59 | + diff_text = ( | ||
| 60 | + "diff --git a/src/scheduler.py b/src/scheduler.py\n" | ||
| 61 | + "--- a/src/scheduler.py\n" | ||
| 62 | + "+++ b/src/scheduler.py\n" | ||
| 63 | + "@@ -1,1 +1,1 @@\n" | ||
| 64 | + "-a\n" | ||
| 65 | + "+b\n" | ||
| 66 | + "diff --git a/frontend/app.js b/frontend/app.js\n" | ||
| 67 | + "--- a/frontend/app.js\n" | ||
| 68 | + "+++ b/frontend/app.js\n" | ||
| 69 | + "@@ -1,1 +1,1 @@\n" | ||
| 70 | + "-a\n" | ||
| 71 | + "+b\n" | ||
| 72 | + ) | ||
| 73 | + self.assertEqual( | ||
| 74 | + core_engine_classify.parse_changed_files_from_diff(diff_text), | ||
| 75 | + ["src/scheduler.py", "frontend/app.js"], | ||
| 76 | + ) | ||
| 77 | + | ||
| 78 | + def test_empty_diff_returns_empty_list(self): | ||
| 79 | + self.assertEqual(core_engine_classify.parse_changed_files_from_diff(""), []) | ||
| 80 | + | ||
| 81 | + def test_none_diff_returns_empty_list(self): | ||
| 82 | + self.assertEqual(core_engine_classify.parse_changed_files_from_diff(None), []) | ||
| 83 | + | ||
| 84 | + | ||
| 85 | +if __name__ == "__main__": | ||
| 86 | + unittest.main() |
src/test_paths.py
| @@ -83,5 +83,33 @@ class IsSeparateFromTest(unittest.TestCase): | @@ -83,5 +83,33 @@ class IsSeparateFromTest(unittest.TestCase): | ||
| 83 | self.assertFalse(paths.is_separate_from(target)) | 83 | self.assertFalse(paths.is_separate_from(target)) |
| 84 | 84 | ||
| 85 | 85 | ||
| 86 | +class RepoRootTest(unittest.TestCase): | ||
| 87 | + def test_matches_computed_repo_root(self): | ||
| 88 | + self.assertEqual(paths.repo_root(), _REPO_ROOT) | ||
| 89 | + | ||
| 90 | + def test_contains_src_and_git(self): | ||
| 91 | + root = paths.repo_root() | ||
| 92 | + self.assertTrue(os.path.isdir(os.path.join(root, "src"))) | ||
| 93 | + self.assertTrue(os.path.isdir(os.path.join(root, ".git"))) | ||
| 94 | + | ||
| 95 | + | ||
| 96 | +class IsSelfTargetTest(unittest.TestCase): | ||
| 97 | + def test_this_repos_own_checkout_is_self_target(self): | ||
| 98 | + self.assertTrue(paths.is_self_target(_REPO_ROOT)) | ||
| 99 | + | ||
| 100 | + def test_other_directory_is_not_self_target(self): | ||
| 101 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 102 | + self.assertFalse(paths.is_self_target(tmp)) | ||
| 103 | + | ||
| 104 | + def test_relative_path_to_repo_root_still_matches(self): | ||
| 105 | + rel = os.path.relpath(_REPO_ROOT, os.getcwd()) | ||
| 106 | + self.assertTrue(paths.is_self_target(rel)) | ||
| 107 | + | ||
| 108 | + def test_explicit_repo_root_override(self): | ||
| 109 | + with tempfile.TemporaryDirectory() as tmp: | ||
| 110 | + self.assertTrue(paths.is_self_target(tmp, repo_root_path=tmp)) | ||
| 111 | + self.assertFalse(paths.is_self_target(_REPO_ROOT, repo_root_path=tmp)) | ||
| 112 | + | ||
| 113 | + | ||
| 86 | if __name__ == "__main__": | 114 | if __name__ == "__main__": |
| 87 | unittest.main() | 115 | unittest.main() |
src/test_scheduler.py
| @@ -17,7 +17,10 @@ import time | @@ -17,7 +17,10 @@ import time | ||
| 17 | import unittest | 17 | import unittest |
| 18 | from unittest.mock import patch | 18 | from unittest.mock import patch |
| 19 | 19 | ||
| 20 | -from src import org_level_loop, org_loop_route, schema, scheduler, settings_config, work_graph_execute, work_node_run | 20 | +from src import ( |
| 21 | + bug_record, e2e_pipeline, org_level_loop, org_loop_route, paths, schema, scheduler, settings_config, | ||
| 22 | + work_graph_execute, work_node_run, | ||
| 23 | +) | ||
| 21 | 24 | ||
| 22 | 25 | ||
| 23 | class FinishTaskLifecycleTestBase(unittest.TestCase): | 26 | class FinishTaskLifecycleTestBase(unittest.TestCase): |
| @@ -50,6 +53,58 @@ class FinishTaskLifecyclePassTest(FinishTaskLifecycleTestBase): | @@ -50,6 +53,58 @@ class FinishTaskLifecyclePassTest(FinishTaskLifecycleTestBase): | ||
| 50 | self.assertEqual(status, "done") | 53 | self.assertEqual(status, "done") |
| 51 | 54 | ||
| 52 | 55 | ||
| 56 | +class FinishTaskLifecycleSelfTargetAuditTest(FinishTaskLifecycleTestBase): | ||
| 57 | + """U-035: self-target PASS closures additively record a core_engine | ||
| 58 | + audit-trail bug_record, without changing close_task_done's own effect.""" | ||
| 59 | + | ||
| 60 | + def test_self_target_core_engine_change_records_audit_bug(self): | ||
| 61 | + run_result = { | ||
| 62 | + "task_id": "task-1", "doc_artifact_ids": {}, "code_artifact_ids": {"code_diff": "art-diff"}, | ||
| 63 | + "verdict": {"verdict": "PASS", "raw_response": "VERDICT: PASS"}, | ||
| 64 | + } | ||
| 65 | + with patch.object(paths, "is_self_target", return_value=True), \ | ||
| 66 | + patch.object(e2e_pipeline, "_read_artifact_content", return_value=( | ||
| 67 | + "diff --git a/src/scheduler.py b/src/scheduler.py\n" | ||
| 68 | + "--- a/src/scheduler.py\n+++ b/src/scheduler.py\n" | ||
| 69 | + )): | ||
| 70 | + result = scheduler.finish_task_lifecycle(self.conn, run_result, "/tmp/proj-1", "goal text") | ||
| 71 | + | ||
| 72 | + self.assertEqual(result["final_status"], "done") | ||
| 73 | + status = self.conn.execute("SELECT status FROM task WHERE task_id = 'task-1'").fetchone()[0] | ||
| 74 | + self.assertEqual(status, "done") # unaffected -- audit trail is additive, not a gate | ||
| 75 | + audit_row = self.conn.execute( | ||
| 76 | + "SELECT root_cause_category, status FROM bug_record WHERE task_id = 'task-1'" | ||
| 77 | + ).fetchone() | ||
| 78 | + self.assertEqual(audit_row, ("core_engine", "resolved")) | ||
| 79 | + | ||
| 80 | + def test_self_target_non_core_engine_change_records_no_audit_bug(self): | ||
| 81 | + run_result = { | ||
| 82 | + "task_id": "task-1", "doc_artifact_ids": {}, "code_artifact_ids": {"code_diff": "art-diff"}, | ||
| 83 | + "verdict": {"verdict": "PASS", "raw_response": "VERDICT: PASS"}, | ||
| 84 | + } | ||
| 85 | + with patch.object(paths, "is_self_target", return_value=True), \ | ||
| 86 | + patch.object(e2e_pipeline, "_read_artifact_content", return_value=( | ||
| 87 | + "diff --git a/frontend/app.js b/frontend/app.js\n" | ||
| 88 | + "--- a/frontend/app.js\n+++ b/frontend/app.js\n" | ||
| 89 | + )): | ||
| 90 | + scheduler.finish_task_lifecycle(self.conn, run_result, "/tmp/proj-1", "goal text") | ||
| 91 | + | ||
| 92 | + count = self.conn.execute("SELECT COUNT(*) FROM bug_record WHERE task_id = 'task-1'").fetchone()[0] | ||
| 93 | + self.assertEqual(count, 0) | ||
| 94 | + | ||
| 95 | + def test_non_self_target_pass_records_no_audit_bug(self): | ||
| 96 | + run_result = { | ||
| 97 | + "task_id": "task-1", "doc_artifact_ids": {}, "code_artifact_ids": {"code_diff": "art-diff"}, | ||
| 98 | + "verdict": {"verdict": "PASS", "raw_response": "VERDICT: PASS"}, | ||
| 99 | + } | ||
| 100 | + # is_self_target NOT patched -- "/tmp/proj-1" is genuinely not this | ||
| 101 | + # repo's own checkout, so _read_artifact_content must never even be | ||
| 102 | + # attempted (would fail on a nonexistent artifact/file otherwise). | ||
| 103 | + scheduler.finish_task_lifecycle(self.conn, run_result, "/tmp/proj-1", "goal text") | ||
| 104 | + count = self.conn.execute("SELECT COUNT(*) FROM bug_record WHERE task_id = 'task-1'").fetchone()[0] | ||
| 105 | + self.assertEqual(count, 0) | ||
| 106 | + | ||
| 107 | + | ||
| 53 | class FinishTaskLifecycleFailTest(FinishTaskLifecycleTestBase): | 108 | class FinishTaskLifecycleFailTest(FinishTaskLifecycleTestBase): |
| 54 | def test_fail_verdict_runs_fix_loop(self): | 109 | def test_fail_verdict_runs_fix_loop(self): |
| 55 | run_result = { | 110 | run_result = { |
src/test_work_graph_orchestrate.py
| @@ -10,7 +10,7 @@ Run: python3 -m unittest src.test_work_graph_orchestrate -v | @@ -10,7 +10,7 @@ Run: python3 -m unittest src.test_work_graph_orchestrate -v | ||
| 10 | import unittest | 10 | import unittest |
| 11 | from unittest.mock import patch | 11 | from unittest.mock import patch |
| 12 | 12 | ||
| 13 | -from src import schema, work_graph_execute, work_graph_orchestrate as wgo | 13 | +from src import paths, schema, work_graph_execute, work_graph_orchestrate as wgo, work_node_run |
| 14 | 14 | ||
| 15 | 15 | ||
| 16 | class TopologicalOrderTest(unittest.TestCase): | 16 | class TopologicalOrderTest(unittest.TestCase): |
| @@ -264,5 +264,47 @@ class VerifyFnTest(RunGraphTestBase): | @@ -264,5 +264,47 @@ class VerifyFnTest(RunGraphTestBase): | ||
| 264 | self.assertEqual(run_status, "blocked") | 264 | self.assertEqual(run_status, "blocked") |
| 265 | 265 | ||
| 266 | 266 | ||
| 267 | +class SelfTargetForcesWorktreeTest(RunGraphTestBase): | ||
| 268 | + """U-035 (05 §5.8): self-target unconditionally forces needs_worktree=True, | ||
| 269 | + regardless of what the caller passed. run_work_node itself is mocked here | ||
| 270 | + (not just execute_work_graph) so this only asserts what needs_worktree | ||
| 271 | + value run_graph computes and passes down -- it doesn't exercise real git | ||
| 272 | + worktree commands against this actual checkout.""" | ||
| 273 | + | ||
| 274 | + def test_self_target_forces_worktree_even_when_caller_passed_false(self): | ||
| 275 | + manifest = {"nodes": [{"node_id": "brd", "depends_on": []}], "edges": []} | ||
| 276 | + self.conn.execute( | ||
| 277 | + "UPDATE project SET target_folder_path = ? WHERE project_id = 'proj-1'", (paths.repo_root(),) | ||
| 278 | + ) | ||
| 279 | + self.conn.commit() | ||
| 280 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 281 | + patch.object(work_node_run, "run_work_node", return_value={"content": "x"}) as mock_run: | ||
| 282 | + wgo.run_graph( | ||
| 283 | + self.conn, "run-1", "planner", "planner-doc-writing", paths.repo_root(), manifest, | ||
| 284 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, needs_worktree=False, | ||
| 285 | + ) | ||
| 286 | + self.assertTrue(mock_run.call_args.kwargs["needs_worktree"]) | ||
| 287 | + | ||
| 288 | + def test_non_self_target_respects_caller_supplied_false(self): | ||
| 289 | + manifest = {"nodes": [{"node_id": "brd", "depends_on": []}], "edges": []} | ||
| 290 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 291 | + patch.object(work_node_run, "run_work_node", return_value={"content": "x"}) as mock_run: | ||
| 292 | + wgo.run_graph( | ||
| 293 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | ||
| 294 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, needs_worktree=False, | ||
| 295 | + ) | ||
| 296 | + self.assertFalse(mock_run.call_args.kwargs["needs_worktree"]) | ||
| 297 | + | ||
| 298 | + def test_non_self_target_still_respects_caller_supplied_true(self): | ||
| 299 | + manifest = {"nodes": [{"node_id": "brd", "depends_on": []}], "edges": []} | ||
| 300 | + with patch.object(work_graph_execute, "write_workflow_script"), \ | ||
| 301 | + patch.object(work_node_run, "run_work_node", return_value={"content": "x"}) as mock_run: | ||
| 302 | + wgo.run_graph( | ||
| 303 | + self.conn, "run-1", "planner", "planner-doc-writing", "/tmp/proj-1", manifest, | ||
| 304 | + lambda node, outputs: "p", _IN_SCHEMA, _OUT_SCHEMA, needs_worktree=True, | ||
| 305 | + ) | ||
| 306 | + self.assertTrue(mock_run.call_args.kwargs["needs_worktree"]) | ||
| 307 | + | ||
| 308 | + | ||
| 267 | if __name__ == "__main__": | 309 | if __name__ == "__main__": |
| 268 | unittest.main() | 310 | unittest.main() |
src/work_graph_orchestrate.py
| @@ -45,7 +45,7 @@ import json | @@ -45,7 +45,7 @@ import json | ||
| 45 | import time | 45 | import time |
| 46 | import uuid | 46 | import uuid |
| 47 | 47 | ||
| 48 | -from src import work_graph_execute, work_node_run | 48 | +from src import paths, work_graph_execute, work_node_run |
| 49 | 49 | ||
| 50 | 50 | ||
| 51 | class ConsistencyCheckFailedError(Exception): | 51 | class ConsistencyCheckFailedError(Exception): |
| @@ -181,6 +181,15 @@ def run_graph( | @@ -181,6 +181,15 @@ def run_graph( | ||
| 181 | writing code (Coder) should pass this; callers only producing document | 181 | writing code (Coder) should pass this; callers only producing document |
| 182 | content (Planner) don't need it. | 182 | content (Planner) don't need it. |
| 183 | 183 | ||
| 184 | + **U-035 (05 §5.8) self-target override**: when `target_folder_path` IS | ||
| 185 | + graphlemon's own checkout (`paths.is_self_target`), `needs_worktree` is | ||
| 186 | + forced True regardless of what the caller passed -- the currently-running | ||
| 187 | + backend process is reading this exact code from disk, so an in-place edit | ||
| 188 | + to the main checkout (even a serial, non-parallel one) risks corrupting | ||
| 189 | + state out from under itself mid-execution. This is unconditional (not | ||
| 190 | + "only when parallel", 03-C §3.6's general rule) precisely because | ||
| 191 | + self-target has no safe non-worktree case at all. | ||
| 192 | + | ||
| 184 | `verify_fn(node, output) -> (ok: bool, feedback)`, if given, is called | 193 | `verify_fn(node, output) -> (ok: bool, feedback)`, if given, is called |
| 185 | right after each node completes, BEFORE moving on to nodes that depend | 194 | right after each node completes, BEFORE moving on to nodes that depend |
| 186 | on it -- lets a caller intercept a bad output before it propagates | 195 | on it -- lets a caller intercept a bad output before it propagates |
| @@ -195,6 +204,7 @@ def run_graph( | @@ -195,6 +204,7 @@ def run_graph( | ||
| 195 | Returns {node_id: output_payload}. | 204 | Returns {node_id: output_payload}. |
| 196 | """ | 205 | """ |
| 197 | ensure_placeholder_skill(conn, skill_id, org_node) | 206 | ensure_placeholder_skill(conn, skill_id, org_node) |
| 207 | + effective_needs_worktree = needs_worktree or paths.is_self_target(target_folder_path) | ||
| 198 | ordered = topological_order(graph_manifest) | 208 | ordered = topological_order(graph_manifest) |
| 199 | outputs = {} | 209 | outputs = {} |
| 200 | for node in ordered: | 210 | for node in ordered: |
| @@ -208,14 +218,14 @@ def run_graph( | @@ -208,14 +218,14 @@ def run_graph( | ||
| 208 | run_fn = _make_run_fn(target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) | 218 | run_fn = _make_run_fn(target_folder_path, node_exec_id, org_node, prompt, output_schema, timeout) |
| 209 | output = work_node_run.run_work_node( | 219 | output = work_node_run.run_work_node( |
| 210 | conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, | 220 | conn, node_exec_id, {"node_id": node_id}, input_schema, output_schema, run_fn, |
| 211 | - needs_worktree=needs_worktree, repo_path=target_folder_path, | 221 | + needs_worktree=effective_needs_worktree, repo_path=target_folder_path, |
| 212 | sleep=sleep, now=now, | 222 | sleep=sleep, now=now, |
| 213 | ) | 223 | ) |
| 214 | 224 | ||
| 215 | if verify_fn is not None: | 225 | if verify_fn is not None: |
| 216 | output = _verify_and_maybe_regenerate( | 226 | output = _verify_and_maybe_regenerate( |
| 217 | conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, | 227 | conn, run_id, org_node, skill_id, target_folder_path, node, node_id, prompt, |
| 218 | - output, input_schema, output_schema, timeout, needs_worktree, verify_fn, sleep, now, | 228 | + output, input_schema, output_schema, timeout, effective_needs_worktree, verify_fn, sleep, now, |
| 219 | ) | 229 | ) |
| 220 | 230 | ||
| 221 | outputs[node_id] = output | 231 | outputs[node_id] = output |