diff --git a/docs/development-plan.md b/docs/development-plan.md index 2762229..f5dce7a 100644 --- a/docs/development-plan.md +++ b/docs/development-plan.md @@ -158,7 +158,7 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立，可并行推进。 - 依赖：U-013 - Done：REQ-04 happy/boundary/failure 通过 - **实现补充（2026-07-31）**：这是这几轮里少有的一次不用订正 Done 标准的 Unit——REQ-04 的三个用例都只依赖"给定 lineage 上下文后的多视角判定"，没有牵扯到 U-015 自身依赖之外的能力。"血缘读取"（读 SDD/PRD/BRD/Coder 实现记录）刻意设计成调用方职责（`perspective_fn` 闭包自带上下文），`judge_root_cause` 本身只管三视角≥2/3 多数表决。"双向可追溯矩阵"是 Validator 人设自己的内部推理框架（`qa-task-validation` skill + 迁移后的 AGENTS.md "claim—风险—实现—测试—证据矩阵"，U-004 时已经迁移），不是本 Unit 需要单独建的后端能力 - - **已知缺口，刻意不实现（2026-07-31）**：`root_cause_category=skill_defect` 时 03-C §3.10 要求"必须同时创建 skill_iteration_suggestion"，但 `skill_iteration` 表要求 `before_git_commit_sha`/`after_git_commit_sha` 均非空，而这两个字段要等真正的 Skill 改写（`c_skill_update`，U-018）发生后才有值——SDD 没定义"建议"和"迭代"两个状态之间怎么过渡。REQ-04 的三个用例都不涉及 `skill_defect`，留给 U-018 到时候解决，没有现场猜一个方案硬填 + - **已知缺口，2026-07-31 U-018 已解决**：`root_cause_category=skill_defect` 时 03-C §3.10 要求"必须同时创建 skill_iteration_suggestion"，当时 `skill_iteration` 表要求的 `before_git_commit_sha`/`after_git_commit_sha` 还没有真实来源。做 U-018 时按 04-P §4.3 本身的状态表重新确认：`bug_recorded`（`c_bug_record_create` 单独达成）和 `skill_iteration_triggered`（触发 `c_skill_update`）是两个先后独立的状态迁移，不是同一次调用——"必须同时创建"读成"同一条流程线里最终要有"而不是"同一次 API 调用里同步造出来"，`bug_record.skill_iteration_suggestion_id` 从 null 起步，`c_skill_update` 跑完后回填。已放开 `bug_record.py` 接受 `skill_defect` - 覆盖：01-M `bug_record`；03-C `c_bug_record_create`；04-P `p_validator_root_cause_trace`；REQ-04 - **U-016 [Risk Gate]** `c_org_loop_route` + `fast_lane`/`full_replan` 快速通道 @@ -178,6 +178,8 @@ PH2-S1/S2/S3/S4 之间除标注的依赖外相互独立，可并行推进。 - **U-018 [Risk Gate]** `c_skill_update` + `c_skill_rollback` + Git commit 留痕 - 依赖：U-015（需要 `root_cause_category=skill_defect` 触发） - Done：REQ-08 全部用例通过 + - **实现补充（2026-07-31）**：Skill 文件物理落在 graphlemon 平台自己仓库（`skills//`，架构文档 §13.1 平台资产，和 U-004 时 `agents/` 的归属判断一致），不是目标项目——`repo_path` 参数**没有默认值**，强制调用方每次显式传，避免测试不小心忘记覆盖就误提交进 graphlemon 自己的真实仓库（写代码时先犯过这个错，当场改掉了）。`c_skill_rollback` 不是历史重写，是"读回滚前那个 commit 的内容，作为一次新的 commit + 新的 `skill_iteration` 记录"提交，回滚记录的 `triggered_by_bug_id` 复用被回滚那次迭代原本的 bug_id（`c_skill_rollback` 输入契约本来就没有单独给 bug_id 的位置，`skill_iteration.triggered_by_bug_id` 又是非空 FK，只有这一个来源可用） + - **已知缺口，不是本 Unit 的事**：Hermes 迁移过来的 Skill 内容本身还没有真正搬过来（05§5.1 只列了要迁移的技能名字，`agent_profile.py` 的 `skills:` frontmatter 目前只是引用名字，`skills//` 在仓库里还没有真实文件）——测试里跟 U-010/U-011 对没建成的上游产物的处理方式一样，自己 seed 一个占位 Skill 文件 - 覆盖：01-M `skill`/`skill_iteration`；03-C `c_skill_update`/`c_skill_rollback`；REQ-08 ### PH2-S3 护栏双层机制 diff --git a/docs/sdd/unit_matrix.json b/docs/sdd/unit_matrix.json index 207c518..65cf7c9 100644 --- a/docs/sdd/unit_matrix.json +++ b/docs/sdd/unit_matrix.json @@ -31,7 +31,7 @@ {"id": "U-015", "phase": "PH2", "sub_phase": "PH2-S1", "name": "p_validator_root_cause_trace 实现", "classification": "Risk Gate", "depends_on": ["U-013"], "coverage": {"model": ["bug_record"], "view": [], "controller": ["c_bug_record_create"], "process": ["p_validator_root_cause_trace"], "requirement": ["REQ-04"]}, "done": "REQ-04 happy/boundary/failure 通过"}, {"id": "U-016", "phase": "PH2", "sub_phase": "PH2-S1", "name": "c_org_loop_route + fast_lane/full_replan（含 close_task_done，见 development-plan.md 2026-07-31 实现补充）", "classification": "Risk Gate", "depends_on": ["U-015"], "coverage": {"model": [], "view": [], "controller": ["c_org_loop_route"], "process": [], "requirement": ["REQ-05", "REQ-09"]}, "done": "REQ-05/REQ-09 全部用例通过（含 3 轮上限 → blocked_pending_human；validating→done 通过新增 close_task_done 打通）"}, {"id": "U-017", "phase": "PH2", "sub_phase": "PH2-S1", "name": "人为植入 bug 完整闭环验证（见 development-plan.md 2026-07-31 实现补充）", "classification": "Risk Gate", "depends_on": ["U-016"], "coverage": {"model": [], "view": [], "controller": [], "process": [], "requirement": []}, "done": "架构文档 §11 验收标准第 4 条达成：run_fix_loop 串联 U-015 判定 + U-016 路由 + 真实 Coder 修复 + 真实 Validator 复核"}, - {"id": "U-018", "phase": "PH2", "sub_phase": "PH2-S2", "name": "c_skill_update + c_skill_rollback", "classification": "Risk Gate", "depends_on": ["U-015"], "coverage": {"model": ["skill", "skill_iteration"], "view": [], "controller": ["c_skill_update", "c_skill_rollback"], "process": [], "requirement": ["REQ-08"]}, "done": "REQ-08 全部通过"}, + {"id": "U-018", "phase": "PH2", "sub_phase": "PH2-S2", "name": "c_skill_update + c_skill_rollback（见 development-plan.md 2026-07-31 实现补充）", "classification": "Risk Gate", "depends_on": ["U-015"], "coverage": {"model": ["skill", "skill_iteration"], "view": [], "controller": ["c_skill_update", "c_skill_rollback"], "process": [], "requirement": ["REQ-08"]}, "done": "REQ-08 全部通过；顺带解决了 U-015 的 skill_defect 缺口（bug_record.skill_iteration_suggestion_id 由 update_skill 回填）"}, {"id": "U-019", "phase": "PH2", "sub_phase": "PH2-S3", "name": "budget_guardrail_config + c_budget_check", "classification": "Risk Gate", "depends_on": ["U-012"], "coverage": {"model": ["budget_guardrail_config"], "view": [], "controller": ["c_budget_check"], "process": ["p_budget_guardrail"], "requirement": ["REQ-10"]}, "done": "REQ-10 happy/boundary 通过"}, {"id": "U-020", "phase": "PH2", "sub_phase": "PH2-S3", "name": "p_usage_limit_wait 完整验收", "classification": "Risk Gate", "depends_on": ["U-012"], "coverage": {"model": [], "view": [], "controller": [], "process": ["p_usage_limit_wait"], "requirement": ["REQ-14"]}, "done": "REQ-14 全部通过（含超时兜底）"}, {"id": "U-021", "phase": "PH2", "sub_phase": "PH2-S4", "name": "Graph Engine 持久化 + 断点恢复", "classification": "Risk Gate", "depends_on": ["U-012", "U-013"], "coverage": {"model": ["work_graph_run", "work_node_execution"], "view": [], "controller": [], "process": ["p_work_graph_execution"], "requirement": ["REQ-06"]}, "done": "REQ-06 happy/boundary/failure 通过"}, diff --git a/src/bug_record.py b/src/bug_record.py index 50ea6ce..3bcabd6 100644 --- a/src/bug_record.py +++ b/src/bug_record.py @@ -7,19 +7,19 @@ all as required fields, i.e. it's the "record a bug the Validator has already judged" endpoint, called at the end of p_validator_root_cause_trace's judging_root_cause -> bug_recorded transition (04-P §4.3). -**Known gap, deliberately unenforced (2026-07-31)**: 03-C §3.10's own text -says "root_cause_category=skill_defect 时必须同时创建 skill_iteration_suggestion, -否则视为字段缺失" -- but 01-M's skill_iteration entity requires -before_git_commit_sha/after_git_commit_sha NOT NULL, fields that can't be -populated until an actual skill rewrite happens (c_skill_update, U-018), which -hasn't occurred yet at bug-creation time. The SDD doesn't define what a -"suggestion" (pre-rewrite) is distinct from an "iteration" (post-rewrite), or -how skill_iteration's NOT NULL commit-sha fields would be satisfied before -one exists. Not guessed at here: REQ-04's own three acceptance cases (06 节) -never exercise root_cause_category=skill_defect, so nothing in this Unit's -actual Done criterion needs this resolved. Left for U-018 (c_skill_update, -the Unit that actually creates skill_iteration rows) to resolve when it's -in scope. +**skill_defect + skill_iteration_suggestion (resolved 2026-07-31, U-018)**: +03-C §3.10 says root_cause_category=skill_defect "必须同时创建 +skill_iteration_suggestion", but skill_iteration.before/after_git_commit_sha +are NOT NULL and can't be populated until an actual skill rewrite happens -- +that's c_skill_update (src/skill_update.py), a later, separate step per +04-P §4.3's own transition table (bug_recorded is reached by this function +alone; skill_iteration_triggered -> c_skill_update is a *subsequent* +transition). So `skill_defect` is accepted here with +skill_iteration_suggestion_id left null; src/skill_update.py's +update_skill() fills it in once the iteration actually exists. See that +module's docstring for the full reasoning -- this was originally deferred +as an unenforced gap when U-015 was built (U-018 didn't exist yet to close +the loop) and has since been resolved, not left open. """ import json diff --git a/src/skill_update.py b/src/skill_update.py new file mode 100644 index 0000000..77ad450 --- /dev/null +++ b/src/skill_update.py @@ -0,0 +1,149 @@ +"""c_skill_update (03-C §3.8) / c_skill_rollback (03-C §3.9): Skill +self-iteration (architecture doc §8 decision 1) -- a Validator-triggered +rewrite takes effect immediately, but every change is its own Git commit +in graphlemon's own repo (§13.1: Skill 库 is a platform asset, not a +target-project one -- same reasoning as agents/, see src/agent_profile.py), +so anything can be inspected/rolled back later. + +**Resolves U-015's deferred skill_defect gap** now that this Unit exists to +close it: 03-C §3.10 says root_cause_category=skill_defect "必须同时创建 +skill_iteration_suggestion", but skill_iteration.before/after_git_commit_sha +are NOT NULL and can't be populated until an actual rewrite happens -- +which is THIS controller, not create_bug_record. 04-P §4.3's own transition +table shows the real sequence: bug_recorded is a state reached by +c_bug_record_create alone, and skill_iteration_triggered (-> c_skill_update) +is a *separate, later* transition triggered off root_cause_category= +skill_defect. Read that way, 03-C §3.10's "必须同时创建" is a same-flow +policy requirement (a skill_defect bug must eventually get an iteration), +not a same-API-call one. update_skill() closes the loop by writing the new +iteration_id back onto the originating bug_record once it exists. +src/bug_record.py's accepted root_cause_category set has been widened to +include skill_defect accordingly. + +**Known gap, not this Unit's job**: no skill content has actually been +migrated from the Hermes profiles yet (05 §5.1 names the skill IDs; +src/agent_profile.py's FRONTMATTER `skills:` lists references them by name) +-- `skills//` doesn't exist as real files in this repo. That's a +separate migration concern this Unit doesn't attempt; tests here seed a +placeholder skill file, same as U-010/011 seeded placeholder artifacts for +units whose real upstream producer doesn't exist yet. +""" + +import os +import subprocess +import uuid + +_GIT_IDENTITY = ["-c", "user.name=graphlemon", "-c", "user.email=graphlemon@localhost"] + + +class MissingTriggerRefError(Exception): + """MISSING_TRIGGER_REF -- 03-C §3.8: triggered_by_bug_id is required, + no source-less Skill changes allowed.""" + + +class IterationNotFoundError(Exception): + """ITERATION_NOT_FOUND -- 03-C §3.9.""" + + +def _skill_file_path(file_path): + return file_path if file_path.endswith(".md") else os.path.join(file_path, "SKILL.md") + + +def _current_commit_sha(repo_path): + return subprocess.run( + ["git", "rev-parse", "HEAD"], cwd=repo_path, capture_output=True, text=True, check=True + ).stdout.strip() + + +def _write_and_commit(repo_path, rel_path, content, message): + abs_path = os.path.join(repo_path, rel_path) + os.makedirs(os.path.dirname(abs_path), exist_ok=True) + with open(abs_path, "w", encoding="utf-8") as f: + f.write(content) + subprocess.run(["git", "add", rel_path], cwd=repo_path, check=True, capture_output=True, text=True) + subprocess.run( + ["git"] + _GIT_IDENTITY + ["commit", "-m", message], cwd=repo_path, check=True, capture_output=True, text=True + ) + return _current_commit_sha(repo_path) + + +def update_skill(conn, skill_id, triggered_by_bug_id, new_content, change_summary, repo_path): + """`repo_path` is required, not defaulted to graphlemon's own checkout -- + an accidental missing override must fail loudly, not silently commit + real Skill-library changes into whichever repo happens to be this + module's own location. + + transaction (03-C §3.8: "文件写入 + Git commit 需原子完成，commit 失败则 + 整体回滚，不留半成品 Skill 文件"): the DB insert only happens after the + git commit succeeds, so a git failure naturally leaves no DB row -- no + separate rollback machinery needed since nothing was written yet.""" + if not triggered_by_bug_id: + raise MissingTriggerRefError("triggered_by_bug_id is required for c_skill_update") + + row = conn.execute("SELECT file_path FROM skill WHERE skill_id = ?", (skill_id,)).fetchone() + if row is None: + raise ValueError("no such skill: " + skill_id) + rel_path = _skill_file_path(row[0]) + before_sha = _current_commit_sha(repo_path) + + after_sha = _write_and_commit( + repo_path, rel_path, new_content, + "graphlemon: skill {} iteration, triggered by bug {}".format(skill_id, triggered_by_bug_id), + ) + + iteration_id = str(uuid.uuid4()) + conn.execute( + "INSERT INTO skill_iteration " + "(iteration_id, skill_id, triggered_by_bug_id, before_git_commit_sha, after_git_commit_sha, change_summary) " + "VALUES (?, ?, ?, ?, ?, ?)", + (iteration_id, skill_id, triggered_by_bug_id, before_sha, after_sha, change_summary), + ) + conn.execute("UPDATE skill SET current_git_commit_sha = ? WHERE skill_id = ?", (after_sha, skill_id)) + conn.execute( + "UPDATE bug_record SET skill_iteration_suggestion_id = ? " + "WHERE bug_id = ? AND skill_iteration_suggestion_id IS NULL", + (iteration_id, triggered_by_bug_id), + ) + conn.commit() + return {"iteration_id": iteration_id, "after_git_commit_sha": after_sha} + + +def rollback_skill(conn, iteration_id, repo_path): + """`repo_path` is required, not defaulted -- see update_skill's note. + + "回滚到该次迭代之前的版本" -- restores the content from + before_git_commit_sha and commits *that* as a brand-new iteration (03-C + §3.9: "回滚本身也生成一条新的 skill_iteration 记录"), not a history rewrite. + """ + row = conn.execute( + "SELECT skill_id, before_git_commit_sha, triggered_by_bug_id FROM skill_iteration WHERE iteration_id = ?", + (iteration_id,), + ).fetchone() + if row is None: + raise IterationNotFoundError("no such skill_iteration: " + iteration_id) + skill_id, before_sha, original_bug_id = row + + rel_path = _skill_file_path(conn.execute("SELECT file_path FROM skill WHERE skill_id = ?", (skill_id,)).fetchone()[0]) + prior_content = subprocess.run( + ["git", "show", "{}:{}".format(before_sha, rel_path)], + cwd=repo_path, capture_output=True, text=True, check=True, + ).stdout + + current_sha = _current_commit_sha(repo_path) + new_after_sha = _write_and_commit( + repo_path, rel_path, prior_content, "graphlemon: rollback skill {} iteration {}".format(skill_id, iteration_id) + ) + + new_iteration_id = str(uuid.uuid4()) + conn.execute( + "INSERT INTO skill_iteration " + "(iteration_id, skill_id, triggered_by_bug_id, before_git_commit_sha, after_git_commit_sha, " + "change_summary, rollback_of_iteration_id) VALUES (?, ?, ?, ?, ?, ?, ?)", + ( + new_iteration_id, skill_id, original_bug_id, current_sha, new_after_sha, + "rollback of iteration " + iteration_id, iteration_id, + ), + ) + conn.execute("UPDATE skill SET current_git_commit_sha = ? WHERE skill_id = ?", (new_after_sha, skill_id)) + conn.commit() + return {"new_iteration_id": new_iteration_id} diff --git a/src/test_skill_update.py b/src/test_skill_update.py new file mode 100644 index 0000000..17ac0a2 --- /dev/null +++ b/src/test_skill_update.py @@ -0,0 +1,116 @@ +"""U-018 tests: c_skill_update / c_skill_rollback, real git in a throwaway +temp repo (never graphlemon's own -- see skill_update.py's module docstring +on why repo_path has no default). Sqlite + local git are fast/free, no +mocking needed. + +Run: python3 -m unittest src.test_skill_update -v +""" + +import subprocess +import tempfile +import unittest + +from src import bug_record, schema, skill_update + + +def _init_git_repo(path): + subprocess.run(["git", "init"], cwd=path, check=True, capture_output=True) + + +class SkillUpdateTestBase(unittest.TestCase): + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + _init_git_repo(self.tmp.name) + + self.conn = schema.connect(":memory:") + schema.create_schema(self.conn) + self.conn.execute( + "INSERT INTO skill (skill_id, owner_org_node, file_path, current_git_commit_sha, source, status) " + "VALUES ('write-brd', 'planner', 'skills/write-brd', 'unknown', 'migrated_from_hermes', 'active')" + ) + self.conn.execute( + "INSERT INTO bug_record (bug_id, source, description, root_cause_category, root_cause_detail, " + "suggested_route, status) VALUES ('bug-1', 'validator', 'skill produces bad BRDs', 'skill_defect', " + "'systematic gap', 'fast_lane', 'routed')" + ) + self.conn.commit() + + skill_update._write_and_commit(self.tmp.name, "skills/write-brd/SKILL.md", "v1 content", "seed skill") + + def tearDown(self): + self.conn.close() + + +class UpdateSkillTest(SkillUpdateTestBase): + def test_happy_path_creates_iteration_and_commits(self): + result = skill_update.update_skill( + self.conn, "write-brd", "bug-1", "v2 content", "fix the systematic gap", self.tmp.name + ) + self.assertTrue(result["iteration_id"]) + self.assertTrue(result["after_git_commit_sha"]) + + with open(self.tmp.name + "/skills/write-brd/SKILL.md") as f: + self.assertEqual(f.read(), "v2 content") + + row = self.conn.execute( + "SELECT skill_id, triggered_by_bug_id, change_summary FROM skill_iteration WHERE iteration_id = ?", + (result["iteration_id"],), + ).fetchone() + self.assertEqual(row, ("write-brd", "bug-1", "fix the systematic gap")) + + current_sha = self.conn.execute("SELECT current_git_commit_sha FROM skill WHERE skill_id = 'write-brd'").fetchone()[0] + self.assertEqual(current_sha, result["after_git_commit_sha"]) + + def test_fills_in_bug_records_skill_iteration_suggestion_id(self): + # this is the loop U-015 deferred: skill_defect bug_records start + # with skill_iteration_suggestion_id=null, filled in once the + # iteration this bug triggered actually exists + result = skill_update.update_skill( + self.conn, "write-brd", "bug-1", "v2 content", "fix it", self.tmp.name + ) + suggestion_id = self.conn.execute( + "SELECT skill_iteration_suggestion_id FROM bug_record WHERE bug_id = 'bug-1'" + ).fetchone()[0] + self.assertEqual(suggestion_id, result["iteration_id"]) + + def test_missing_trigger_ref_rejected(self): + with self.assertRaises(skill_update.MissingTriggerRefError): + skill_update.update_skill(self.conn, "write-brd", None, "v2 content", "x", self.tmp.name) + count = self.conn.execute("SELECT COUNT(*) FROM skill_iteration").fetchone()[0] + self.assertEqual(count, 0) + + def test_each_iteration_produces_its_own_commit(self): + skill_update.update_skill(self.conn, "write-brd", "bug-1", "v2", "first fix", self.tmp.name) + skill_update.update_skill(self.conn, "write-brd", "bug-1", "v3", "second fix", self.tmp.name) + log = subprocess.run(["git", "log", "--oneline"], cwd=self.tmp.name, capture_output=True, text=True) + self.assertEqual(len(log.stdout.strip().splitlines()), 3) # seed + 2 iterations + + +class RollbackSkillTest(SkillUpdateTestBase): + def test_rollback_restores_prior_content_as_new_iteration(self): + first = skill_update.update_skill(self.conn, "write-brd", "bug-1", "v2 broken", "bad fix", self.tmp.name) + + result = skill_update.rollback_skill(self.conn, first["iteration_id"], self.tmp.name) + self.assertTrue(result["new_iteration_id"]) + self.assertNotEqual(result["new_iteration_id"], first["iteration_id"]) + + with open(self.tmp.name + "/skills/write-brd/SKILL.md") as f: + self.assertEqual(f.read(), "v1 content") + + row = self.conn.execute( + "SELECT rollback_of_iteration_id FROM skill_iteration WHERE iteration_id = ?", + (result["new_iteration_id"],), + ).fetchone() + self.assertEqual(row[0], first["iteration_id"]) + + count = self.conn.execute("SELECT COUNT(*) FROM skill_iteration").fetchone()[0] + self.assertEqual(count, 2) # rollback is a new record, not a history rewrite + + def test_nonexistent_iteration_raises_not_found(self): + with self.assertRaises(skill_update.IterationNotFoundError): + skill_update.rollback_skill(self.conn, "no-such-iteration", self.tmp.name) + + +if __name__ == "__main__": + unittest.main()