diff --git a/docs/todo/graph-engineering-open-items.md b/docs/todo/graph-engineering-open-items.md index a7ef909..a3c8276 100644 --- a/docs/todo/graph-engineering-open-items.md +++ b/docs/todo/graph-engineering-open-items.md @@ -120,7 +120,7 @@ ## H. 统一 Role Package 抽象层 -**状态**：✅ 已确认（需求评审通过，2026-08-06）→ 需求文档 `docs/todo/role-package-abstraction-requirements.md`（注：文档正文里自己写的路径少了 `todo/` 一段，实际文件在这个路径下） +**状态**：✅ 已实现（2026-08-06）→ 需求文档 `docs/todo/role-package-abstraction-requirements.md` + 设计文档 `docs/todo/role-package-abstraction-design.md` **来源**：不属于三份 Graph Engineering 对照笔记，是用户对照 Agentic Enterprise 产品目标（"role + context + skills + applications + workflows + permissions + acceptance criteria；all essentially are context for intelligent computing"）单独提出的差距检查，2026-08-06。 @@ -133,6 +133,17 @@ 其余判断（`role` 表外键 vs 自由文本+应用层校验、applications 维度最小定义、迁移步骤、workflows 字段怎么接 E）维持文档第 8 节"留待详细设计阶段回答"不变，本次评审不抢先拍板。 +**实现结果（2026-08-06，FR-3 范围问题跟用户确认过——4 张表一次性全改，不分阶段）**： + +- **载体格式选了 JSON,不是 YAML**：05 §5.7 纯 stdlib 约束排除了 PyYAML；Skill 文件的 YAML frontmatter 之所以能用是因为 graphlemon 自己的代码从来不解析它（只有 Claude Code 自己解析），但 Role Package 需要被 `agent_profile.py` 真正读懂，手写 YAML 子集解析器风险高，`json.load` 零风险。`roles/{planner,coder,validator}/role.json` 三个文件落地，覆盖七个维度。 +- **role.json 是"引用清单",不是内容复制**：`identity.soul_md_path`/`agents_md_path` 原样指向没动过的 `agents//SOUL.md`/`AGENTS.md`——这个决定让"部署产物必须不变"这条验收标准（标准 2）几乎自动成立：`render_agent_definition()` 的 body 拼接逻辑一行没变，只有 frontmatter 字段的来源从硬编码字典变成解析 `role.json`。**逐字节验证过**：直接用 `git show HEAD:src/agent_profile.py` 跑旧代码、跟新代码的 `render_agent_definition()` 输出做 diff，三个角色全部逐字节相同。 +- **`agent_profile.py` 改造**：`ORG_NODES` 从硬编码三元组变成扫描 `roles/` 目录发现（`_discover_role_ids()`），`FRONTMATTER`/`_DESCRIPTIONS`/`_MIGRATED_FROM_PROFILE` 三个硬编码字典全部删除。三个公开函数都加了 `roles_dir` 可选参数，专门用来在测试里证明"新增角色不用改代码"——测试往临时目录塞一个新角色的 `role.json`，直接调用这三个**未经修改**的函数，确认新角色能被发现、渲染、部署（`NewRoleWithoutCodeChangeTest`）。 +- **Schema：4 张表全改**——`skill.owner_org_node`/`task.assigned_org_node`/`work_graph_run.org_node`/`agent_profile.org_node` 的 `CHECK (x IN ('planner','coder','validator'))` 全部放开成 `CHECK (x != '')`。评审阶段发现的真实改动面比原始风险描述更细：`task`/`work_graph_run`/`skill` 被多张表外键引用，但因为改的是父表、子表的 `REFERENCES` 定义本身不需要变，只需要重建这 4 张父表本身，不需要动任何子表——按 SQLite 官方文档给的流程（`PRAGMA foreign_keys=OFF` → 重建 → `PRAGMA foreign_key_check` 校验无悬空外键 → `PRAGMA foreign_keys=ON`）写了 `migrate_relax_org_node_constraints()`，依赖 D 的 `migrate_add_work_graph_fork_columns()` 先跑（内部会自动调用，幂等）。`ux_work_graph_run_active`（D 依赖的分区唯一索引）、`task` 的三个索引，重建后都验证过仍然生效。 +- **顺手修了评审阶段发现的遗漏依赖**：`work_graph_run_query.py::_VALID_ORG_NODES` 改成直接引用 `agent_profile.ORG_NODES`，不再自己维护一份硬编码三元组。 +- **明确没做的部分（如实记录，评审阶段已经说清楚边界）**：`frontend/app.js` 的 4 处硬编码三角色枚举（路由正则、排序数组、下拉列表、查找对象）本轮不改——新增一个角色之后 PH3 控制台依然认不出它，这是刻意保留的已知缺口，不是遗漏；`role` 表外键完整性方案本轮也没做，用的是更简单的"放开枚举"。 + +`python3 -m unittest discover -s src`（623 tests）+ `discover -s supervisor`（125 tests）全绿，零回归。至此 Graph Engineering 系列 A/B/D/E/F/G/H 全部完成，C 已确认不做。 + --- ## 工作量与优先级评估（2026-08-06） @@ -143,7 +154,7 @@ | **E** 动态 Work Graph 设计能力 | 让 Coder/Validator 也能规划自己的图，Planning 提示词识别可并行节点 | ✅ **已实现**（2026-08-06，580 测试全绿） | L | 已完成 | G（已满足） | | **F** 三视角评审团 fan-out | `judge_root_cause` 从 Python 函数改成 Work Graph 菱形拓扑 | ✅ **已实现**（2026-08-06，586 测试全绿） | M | 已完成 | 无（G 已满足） | | **B** loop-until-dry Skill | Validator 未知规模扫描收敛能力 | ✅ **已实现**（2026-08-06，587 测试全绿） | S/M | 已完成 | 无 | -| **H** Role Package 抽象层 | 统一 role/context/skills/applications/workflows/permissions/acceptance 打包格式 | ✅ 需求评审通过（2026-08-06），待设计 | XL | P2（战略级，不阻塞其他项） | workflows 字段依赖 E 的产出（已满足） | +| **H** Role Package 抽象层 | 统一 role/context/skills/applications/workflows/permissions/acceptance 打包格式 | ✅ **已实现**（2026-08-06，623 测试全绿） | XL | 已完成 | 无（E 已满足） | | **D** Time-travel 调试 | 回到历史节点分叉重跑 | ✅ **已实现**（2026-08-06，608 测试全绿） | L | 已完成 | 无 | | C 按节点选模型 | — | ⛔ 已确认不做 | — | — | — | | A 术语对照 | — | ✅ 已完成 | — | — | — | diff --git a/docs/todo/role-package-abstraction-design.md b/docs/todo/role-package-abstraction-design.md new file mode 100644 index 0000000..44805be --- /dev/null +++ b/docs/todo/role-package-abstraction-design.md @@ -0,0 +1,111 @@ +# 统一 Role Package 抽象层——详细设计（SDD） + +**日期**: 2026-08-06 +**性质**: 详细设计，对应需求文档 `docs/todo/role-package-abstraction-requirements.md`（条目 H，需求评审已通过，见 `docs/todo/graph-engineering-open-items.md` H 节的评审记录）。回答该需求文档第 8 节的开放问题。 +**范围确认（用户 2026-08-06）**：FR-3 的 4 张 CHECK 约束表一次性全改，不分阶段。 +**状态**: 设计草案，实施中。 + +## 1. Role Package 载体格式：JSON，不是 YAML + +需求文档 FR-2 的开放问题是"单文件 vs 目录约定"，但更根本的约束是 05 §5.7 的纯 stdlib 限制——graphlemon 自己的代码不能依赖 PyYAML。`skills//SKILL.md` 现有的 YAML frontmatter 之所以能用，是因为**graphlemon 自己的代码从来没有解析过它**（`skill_query.py`/`skill_update.py` 只把 SKILL.md 当不透明文本读写，真正解析 `skills:`/`tools:` frontmatter 的是 Claude Code 自己，不是这个仓库的 Python 代码）。Role Package 不一样：`agent_profile.py` 需要真正**读懂**它的结构化字段（哪些 skills、哪些 tools、memory 配置……），手写一个 YAML 子集解析器风险高、容易在多行字符串/嵌套列表这类边界情况上出 bug——纯 stdlib 的 `json` 模块能零风险做到同样的事。 + +**决定**：`roles//role.json`，纯 JSON，`json.load` 直接解析。 + +## 2. role.json 是"引用清单"，不是内容复制 + +需求文档 FR-1 自己的措辞已经暗示了这一点——"context（memory 配置 + 运行时 context 组装规则的**引用**）"、"skills（**对应**现有 `skills:` frontmatter）"、"acceptance criteria（**引用**）"——七个维度里大多数本来就该是指针，不是把内容整个复制一份。 + +具体到 role/identity 维度：`agents//SOUL.md`（+ validator 的 `AGENTS.md`）**原样保留，不删除、不合并、不改名**——role.json 只存这两个文件的相对路径引用（`identity.soul_md_path`/`identity.agents_md_path`）。这个决定直接让验收标准 2（"迁移后部署产物逐字节一致"）几乎自动成立：`render_agent_definition()` 的 body 拼接逻辑（`SOUL.md` 内容 + `AGENTS.md` 内容）完全不变，唯一变化的是 frontmatter 字段的**来源**从硬编码 `FRONTMATTER` 字典变成解析 `role.json`。 + +```json +{ + "role_id": "validator", + "description": "graphlemon Validator: independently verifies a task, PASS/FAIL/INCONCLUSIVE, read-only on source.", + "migrated_from_profile": "qa", + "identity": { + "soul_md_path": "agents/validator/SOUL.md", + "agents_md_path": "agents/validator/AGENTS.md" + }, + "context": { "memory": "project" }, + "model": "claude-sonnet-5", + "skills": ["qa-task-validation", "unattended-agent-operations", "design-dynamic-work-graph", "unknown-scope-defect-sweep"], + "applications": { "mcp_servers": [] }, + "workflows": { "dynamic_planning": "generic" }, + "permissions": { + "tools": ["Read", "Bash", "Grep", "Glob"], + "disallowed_tools": ["Edit", "Write"], + "notes": [ + { "constraint": "不提交、推送、reset、清理或覆盖用户工作树", "enforced": "prose_only" } + ] + }, + "acceptance_criteria": { "ref": "docs/sdd/06-验收与测试锚点.md" } +} +``` + +- **applications**（FR-6，"应该"）：`mcp_servers` 列表，允许为空——满足"这个维度存在、字段长什么样、允许为空"，不要求真实集成（非目标 #4）。 +- **workflows**：接 E 的产出（非目标 #2 明确不重复设计 E 已经做的事，这里只是一个指向枚举）。`planner` 是 `"canonical_doc_chain"`（`work_graph_plan.py` 里 `org_node == "planner"` 走的固定链），`coder`/`validator` 是 `"generic"`（E 的 `_GENERIC_PROMPT_TEMPLATE`/`generic_node_prompt_builder` 路径）。 +- **permissions.notes**（验收标准 4）：Validator 的 Bash 子命令限定失效那条已知漏洞，诚实标注 `"enforced": "prose_only"`，不假装是工具层强制——非目标 #3 明确不在本文档修复这个漏洞本身，只要求如实反映现状。 + +## 3. `agent_profile.py` 改造：`ORG_NODES` 从硬编码元组变成目录发现 + +```python +_ROLES_SOURCE_DIR = os.path.join(_REPO_ROOT, "roles") + +def _discover_role_ids(roles_dir=None): + """扫描 roles//role.json 是否存在，role_id 就是目录名。 + roles_dir 可覆盖（测试用真实文件系统 fixture 验证'新增角色不用改代码' + 这条验收标准，不需要真的往这个仓库的 roles/ 目录里塞测试数据）。""" + roles_dir = roles_dir or _ROLES_SOURCE_DIR + if not os.path.isdir(roles_dir): + return () + return tuple(sorted( + name for name in os.listdir(roles_dir) + if os.path.isfile(os.path.join(roles_dir, name, "role.json")) + )) + +ORG_NODES = _discover_role_ids() +``` + +`FRONTMATTER`/`_DESCRIPTIONS`/`_MIGRATED_FROM_PROFILE` 三个硬编码字典全部删除，替换成 `_load_role(role_id, roles_dir=None)`（`json.load` 解析对应 `role.json`）。`render_agent_definition()`/`deploy_agent_definitions()`/`record_agent_profiles()` 都接受一个可选 `roles_dir` 参数（默认 `None` 即用真实目录），这是验收标准 1 能被自动化测试验证的关键——测试往一个临时目录写一个新角色的 `role.json`，不改任何 Python 代码或 `roles_dir` 之外的任何东西，直接调用这三个未经修改的函数，确认新角色能被发现、渲染、部署。 + +`ORG_NODES` 这个名字保留不改（`org_node_trigger.py` 等外部模块已经在用 `from src.agent_profile import ORG_NODES`，改名会不必要地扩大改动面），但注释说明它现在是数据驱动的，不再是硬编码元组。 + +## 4. Schema：4 张表的 CHECK 约束怎么改 + +**决定的形状**：`CHECK (x IN ('planner', 'coder', 'validator'))` → `CHECK (x != '')`——仍然是数据库层面的最小完整性保证（非空字符串），但不再枚举具体值。不是完全去掉 CHECK（那样连"总不能是空字符串"这点基本保证都没有了），也不是新建一张 `role` 表做外键（需求文档 §8 留的另一个选项）——本设计选择更简单的那条路径：4 张表全改，但改的目标是"放开枚举"而不是"建立引用完整性"，避免这次改动同时背上"外键完整性设计"这个新的复杂度。 + +**真实的改动面**（这是这次评审最重要的发现，比需求文档原始风险描述更细）：`task`/`work_graph_run`/`skill` 三张表被 `work_node_execution`/`artifact`/`bug_record`/`execution_event` 等多张表通过外键引用；`agent_profile` 没有任何表引用它。SQLite 不支持 `ALTER TABLE ... DROP CONSTRAINT`，改 CHECK 约束必须用"建新表复制数据删旧表改名"的标准操作——但因为改的是**父表**、外键定义本身（`REFERENCES task(task_id)` 这类文本）**不需要跟着变**，所以只需要重建这 4 张父表本身，不需要动任何一张子表。SQLite 官方文档给的正确流程（"Making Other Kinds Of Table Schema Changes"）是： + +1. `PRAGMA foreign_keys = OFF`（关闭本连接的外键校验，只在这次迁移期间） +2. 开事务 +3. 对每张表：建同结构、新 CHECK 的 `_new` → `INSERT INTO
_new SELECT * FROM
` → `DROP TABLE
` → `ALTER TABLE
_new RENAME TO
` → 重建这张表原有的索引 +4. `PRAGMA foreign_key_check` 确认没有产生悬空外键 +5. 提交事务 +6. `PRAGMA foreign_keys = ON` + +`migrate_relax_org_node_constraints(conn)` 一个函数按这个流程处理全部 4 张表（一次性开一个事务，不是 4 个独立迁移各自开关 `foreign_keys`），每张表重建前用 `sqlite_master` 里的 CREATE TABLE 文本判断是否已经是新约束，已经是就跳过（幂等，`migrate_expand_artifact_types` 的同款前置检查模式）。`SCHEMA_SQL` 里的 4 处 `CHECK (... IN ('planner', 'coder', 'validator'))` 直接改成 `CHECK (... != '')`，新数据库不需要迁移。 + +**索引重建清单**（重建表之后必须重新建的，`DROP TABLE` 会连带丢失表自己的索引）： +- `skill`：无额外索引。 +- `task`：`idx_task_status`、`idx_task_assigned_org_node`、`idx_task_project_id`。 +- `work_graph_run`：`ux_work_graph_run_active`（分区唯一索引，D 已经依赖这条索引的语义，重建后必须原样保留）。 +- `agent_profile`：PK 本身即索引，无需额外重建。 + +## 5. `work_graph_run_query.py::_VALID_ORG_NODES`（评审时发现的遗漏依赖） + +改成直接引用 `agent_profile.ORG_NODES`，不再自己维护一份硬编码三元组——这条不在需求文档原始 FR 列表里，是评审阶段核实代码时发现的真实遗漏（`docs/todo/graph-engineering-open-items.md` H 节评审记录已经点名），顺手一起改掉。 + +## 6. 明确不做的部分（如实记录，呼应评审阶段的结论） + +- **`frontend/app.js` 的 4 处硬编码三角色枚举**（路由正则、`ORG_NODE_ORDER` 排序数组、任务创建表单下拉、`byOrgNode` 查找对象）——评审阶段已经确认这些不在验收标准 1 的范围内（验收标准 1 明确"这个新角色不需要真的接入 Org Graph 执行"），本轮不改。新增一个角色之后，PH3 控制台依然认不出它，这是本轮已知、刻意保留的缺口，不是遗漏。 +- **`role` 表 + 外键完整性**（§4 提到的另一个选项）——本轮用更简单的"放开枚举"实现，不建立真正的引用完整性约束。 +- **`org_node_trigger.py` 校验逻辑本身**——它已经用 `from src.agent_profile import ORG_NODES` 做校验，`ORG_NODES` 变成数据驱动之后这条校验自动跟着数据驱动，不需要改这个文件。 +- **permissions 维度的 Bash 子命令限定漏洞本身**——诚实标注（§2），不修复（需求文档非目标 #3）。 + +## 7. 回答需求文档第 8 节开放问题 + +- 载体格式：单文件 `role.json`（不是目录多文件），JSON 不是 YAML（§1）。 +- CHECK 约束改造方式：放开成 `!= ''`，不建 `role` 表外键（§4）。 +- applications 最小定义：`{"mcp_servers": []}`（§2）。 +- 迁移步骤：三个角色各自的 `role.json` 直接引用现有 `agents//SOUL.md`/`AGENTS.md`，不复制内容（§2），验证方式是 `test_agent_profile.py` 现有测试断言部署产物不变。 +- Role Package 跟 E 的接口：`workflows.dynamic_planning` 字段，`"canonical_doc_chain"` / `"generic"` 二选一（§2）。 diff --git a/roles/coder/role.json b/roles/coder/role.json new file mode 100644 index 0000000..948ed0e --- /dev/null +++ b/roles/coder/role.json @@ -0,0 +1,34 @@ +{ + "role_id": "coder", + "description": "graphlemon Coder: implements one task's code + tests against the Planner's SDD.", + "migrated_from_profile": "code", + "identity": { + "soul_md_path": "agents/coder/SOUL.md", + "agents_md_path": null + }, + "context": { + "memory": "project" + }, + "model": "claude-sonnet-5", + "skills": [ + "code-task-development", + "code-agent-engineering", + "debugging-toolkit", + "software-planning-execution", + "design-dynamic-work-graph" + ], + "applications": { + "mcp_servers": [] + }, + "workflows": { + "dynamic_planning": "generic" + }, + "permissions": { + "tools": ["Read", "Write", "Edit", "Bash", "Grep", "Glob"], + "disallowed_tools": [], + "notes": [] + }, + "acceptance_criteria": { + "ref": "docs/sdd/06-验收与测试锚点.md" + } +} diff --git a/roles/planner/role.json b/roles/planner/role.json new file mode 100644 index 0000000..51fe87e --- /dev/null +++ b/roles/planner/role.json @@ -0,0 +1,35 @@ +{ + "role_id": "planner", + "description": "graphlemon Planner: turns a goal into BRD/PRD/SDD/Benchmark artifacts. Never writes business code.", + "migrated_from_profile": "planning", + "identity": { + "soul_md_path": "agents/planner/SOUL.md", + "agents_md_path": null + }, + "context": { + "memory": "project" + }, + "model": "claude-sonnet-5", + "skills": [ + "planning-requirement-expansion", + "project-brd-planning", + "project-prd-planning", + "project-sdd-planning", + "project-unit-decomposition", + "review-blocker-verification" + ], + "applications": { + "mcp_servers": [] + }, + "workflows": { + "dynamic_planning": "canonical_doc_chain" + }, + "permissions": { + "tools": ["Read", "Write", "Grep", "Glob"], + "disallowed_tools": [], + "notes": [] + }, + "acceptance_criteria": { + "ref": "docs/sdd/06-验收与测试锚点.md" + } +} diff --git a/roles/validator/role.json b/roles/validator/role.json new file mode 100644 index 0000000..cdd6ec5 --- /dev/null +++ b/roles/validator/role.json @@ -0,0 +1,39 @@ +{ + "role_id": "validator", + "description": "graphlemon Validator: independently verifies a task, PASS/FAIL/INCONCLUSIVE, read-only on source.", + "migrated_from_profile": "qa", + "identity": { + "soul_md_path": "agents/validator/SOUL.md", + "agents_md_path": "agents/validator/AGENTS.md" + }, + "context": { + "memory": "project" + }, + "model": "claude-sonnet-5", + "skills": [ + "qa-task-validation", + "unattended-agent-operations", + "design-dynamic-work-graph", + "unknown-scope-defect-sweep" + ], + "applications": { + "mcp_servers": [] + }, + "workflows": { + "dynamic_planning": "generic" + }, + "permissions": { + "tools": ["Read", "Bash", "Grep", "Glob"], + "disallowed_tools": ["Edit", "Write"], + "notes": [ + { + "constraint": "不提交、推送、reset、清理或覆盖用户工作树", + "enforced": "prose_only", + "detail": "scoped Bash(...) disallowedTools entries silently disable Bash entirely in Claude Code subagent frontmatter (confirmed 2026-07-31, see agent_profile.py's prior FRONTMATTER comment) -- this constraint relies on AGENTS.md prose, not a tool-level block." + } + ] + }, + "acceptance_criteria": { + "ref": "docs/sdd/06-验收与测试锚点.md" + } +} diff --git a/src/agent_profile.py b/src/agent_profile.py index d7c5e86..a696eaf 100644 --- a/src/agent_profile.py +++ b/src/agent_profile.py @@ -19,92 +19,66 @@ agent_profile (01-M 1.9) is a singleton per org_node, has no target-project reference -- it tracks graphlemon's own canonical definition, not a per-deploy record, so deploy_agent_definitions() doesn't touch it; record_agent_profiles() does, separately. + +**H (2026-08-06, Role Package abstraction)**: what used to be two hardcoded +Python dicts (`FRONTMATTER`/`_DESCRIPTIONS`/`_MIGRATED_FROM_PROFILE`) is now +`roles//role.json` -- a declarative manifest, not code. `ORG_NODES` +is no longer a fixed 3-tuple; it's discovered by scanning `roles/` for +directories containing a `role.json`, so a new role becomes recognized by +adding one JSON file, no schema.py/agent_profile.py edit. role.json is JSON, +not YAML, despite Skill files using YAML frontmatter -- pure-stdlib (05 +§5.7) rules out PyYAML, and unlike Skill frontmatter (which only Claude Code +itself ever parses, never this codebase), role.json genuinely needs to be +read by this module's own Python code; `json.load` is the zero-risk choice, +no hand-rolled parser. role.json content is deliberately a REFERENCE +manifest, not a content copy: `identity.soul_md_path`/`agents_md_path` still +point at the same `agents//SOUL.md`/`AGENTS.md` files, unmoved -- +see docs/todo/role-package-abstraction-design.md §2 for why keeping the +existing identity files as the single source of prose content makes the +migration's "deployed output must stay byte-identical" requirement close to +automatic (only the frontmatter's data SOURCE changed, not the body-assembly +logic). Every public function here takes an optional `roles_dir` override +(defaults to the real `roles/` directory) precisely so a test can prove "a +new role needs no code change" against a throwaway fixture directory instead +of mutating this repo's own `roles/`. """ +import json import os _REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) _AGENTS_SOURCE_DIR = os.path.join(_REPO_ROOT, "agents") +_ROLES_SOURCE_DIR = os.path.join(_REPO_ROOT, "roles") + + +def _discover_role_ids(roles_dir=None): + """role_id == directory name, for every `//role.json` + that exists. Sorted for deterministic iteration order (matches the old + hardcoded tuple's alphabetical `("coder", "planner", "validator")`... + actually the old tuple was declared as ("planner", "coder", "validator"), + not alphabetical -- callers that relied on that exact order didn't exist + (grep confirms every real use is either membership-testing or a `for` + loop that doesn't care about order), so sorted() here is a deliberate, + harmless simplification, not a compatibility requirement).""" + roles_dir = roles_dir or _ROLES_SOURCE_DIR + if not os.path.isdir(roles_dir): + return () + return tuple(sorted( + name for name in os.listdir(roles_dir) + if os.path.isfile(os.path.join(roles_dir, name, "role.json")) + )) + + +ORG_NODES = _discover_role_ids() -ORG_NODES = ("planner", "coder", "validator") - -_MIGRATED_FROM_PROFILE = { - "planner": "planning", - "coder": "code", - "validator": "qa", -} - -_DESCRIPTIONS = { - "planner": "graphlemon Planner: turns a goal into BRD/PRD/SDD/Benchmark artifacts. Never writes business code.", - "coder": "graphlemon Coder: implements one task's code + tests against the Planner's SDD.", - "validator": "graphlemon Validator: independently verifies a task, PASS/FAIL/INCONCLUSIVE, read-only on source.", -} - -# 05 §5.1: skills:/memory:/tools:/model: (and, for validator, disallowedTools: -- -# the qa/AGENTS.md workspace-permission-contract folded in) are the frontmatter -# fields U-004's Done criterion checks for. -FRONTMATTER = { - "planner": { - "memory": "project", - "model": "claude-sonnet-5", - "skills": [ - "planning-requirement-expansion", - "project-brd-planning", - "project-prd-planning", - "project-sdd-planning", - "project-unit-decomposition", - "review-blocker-verification", - ], - "tools": ["Read", "Write", "Grep", "Glob"], - }, - "coder": { - "memory": "project", - "model": "claude-sonnet-5", - "skills": [ - "code-task-development", - "code-agent-engineering", - "debugging-toolkit", - "software-planning-execution", - "design-dynamic-work-graph", - ], - "tools": ["Read", "Write", "Edit", "Bash", "Grep", "Glob"], - }, - "validator": { - "memory": "project", - "model": "claude-sonnet-5", - "skills": [ - "qa-task-validation", "unattended-agent-operations", "design-dynamic-work-graph", - "unknown-scope-defect-sweep", - ], - "tools": ["Read", "Bash", "Grep", "Glob"], - # 2026-07-31 fix, found for real running U-014's full closed loop: - # scoped Bash patterns here ("Bash(git commit:*)") don't just block - # that one subcommand -- they silently disable Bash *entirely* for - # the agent (confirmed by isolating it: a plain ["Edit","Write"] - # disallowedTools list leaves Bash working fine; adding any - # "Bash(...)" entry breaks it, even though the CLI's own - # --disallowedTools flag documents this exact scoped syntax as - # supported -- it just doesn't work the same way in subagent - # frontmatter). A Validator that can't run tests at all is a much - # worse outcome than the residual risk of it running `git commit`, - # so the git-mutation restriction now relies on the prose instruction - # already in the migrated AGENTS.md ("不提交、推送、reset、清理或覆盖用户 - # 工作树") instead of a tool-level block that doesn't actually work. - # ponytail: this is a real gap (no enforced ceiling on Bash git - # mutations, just an asked-nicely one) -- upgrade path is whatever - # Claude Code's correct scoped-disallowedTools frontmatter syntax - # turns out to be, if this gets fixed/documented upstream. - "disallowedTools": ["Edit", "Write"], - }, -} - - -def _soul_path(org_node): - return os.path.join(_AGENTS_SOURCE_DIR, org_node, "SOUL.md") - - -def _agents_md_path(org_node): - return os.path.join(_AGENTS_SOURCE_DIR, org_node, "AGENTS.md") + +def _role_json_path(role_id, roles_dir=None): + return os.path.join(roles_dir or _ROLES_SOURCE_DIR, role_id, "role.json") + + +def _load_role(role_id, roles_dir=None): + with open(_role_json_path(role_id, roles_dir), encoding="utf-8") as f: + return json.load(f) def _read(path): @@ -112,46 +86,59 @@ def _read(path): return f.read() -def _frontmatter_yaml(org_node, fm): - lines = ["---", f"name: {org_node}", f"description: {_DESCRIPTIONS[org_node]}"] - lines.append(f"memory: {fm['memory']}") - lines.append(f"model: {fm['model']}") +def _frontmatter_yaml(role): + # 05 §5.1: skills:/memory:/tools:/model: (and, for validator, + # disallowedTools: -- the qa/AGENTS.md workspace-permission-contract + # folded in) are the frontmatter fields U-004's Done criterion checks + # for. role.json's own field names are snake_case (permissions.tools/ + # disallowed_tools); the deployed frontmatter keeps Claude Code's own + # camelCase `disallowedTools:` key -- this function is the one place + # that translates between the two, not role.json itself. + permissions = role["permissions"] + lines = ["---", "name: {}".format(role["role_id"]), "description: {}".format(role["description"])] + lines.append("memory: {}".format(role["context"]["memory"])) + lines.append("model: {}".format(role["model"])) lines.append("skills:") - lines += [f" - {s}" for s in fm["skills"]] + lines += [" - {}".format(s) for s in role["skills"]] lines.append("tools:") - lines += [f" - {t}" for t in fm["tools"]] - if "disallowedTools" in fm: + lines += [" - {}".format(t) for t in permissions["tools"]] + if permissions.get("disallowed_tools"): lines.append("disallowedTools:") - lines += [f" - {t}" for t in fm["disallowedTools"]] + lines += [" - {}".format(t) for t in permissions["disallowed_tools"]] lines.append("---") return "\n".join(lines) -def render_agent_definition(org_node): +def render_agent_definition(org_node, roles_dir=None): """Assemble the .claude/agents/.md content: frontmatter + migrated body.""" - if org_node not in ORG_NODES: + if org_node not in _discover_role_ids(roles_dir): raise ValueError(org_node) - body = _read(_soul_path(org_node)) - agents_md_path = _agents_md_path(org_node) - if os.path.exists(agents_md_path): - body += "\n\n" + _read(agents_md_path) - return _frontmatter_yaml(org_node, FRONTMATTER[org_node]) + "\n\n" + body - - -def deploy_agent_definitions(target_folder_path): - """Write .claude/agents/{planner,coder,validator}.md into a target project.""" + role = _load_role(org_node, roles_dir) + identity = role["identity"] + body = _read(os.path.join(_REPO_ROOT, identity["soul_md_path"])) + if identity.get("agents_md_path"): + body += "\n\n" + _read(os.path.join(_REPO_ROOT, identity["agents_md_path"])) + return _frontmatter_yaml(role) + "\n\n" + body + + +def deploy_agent_definitions(target_folder_path, roles_dir=None): + """Write .claude/agents/.md into a target project for every + discovered role (still exactly {planner, coder, validator} against this + repo's real roles/ directory, but data-driven now -- see module + docstring).""" out_dir = os.path.join(target_folder_path, ".claude", "agents") os.makedirs(out_dir, exist_ok=True) - for org_node in ORG_NODES: + for org_node in _discover_role_ids(roles_dir): with open(os.path.join(out_dir, org_node + ".md"), "w", encoding="utf-8") as f: - f.write(render_agent_definition(org_node)) + f.write(render_agent_definition(org_node, roles_dir)) -def record_agent_profiles(conn, current_git_commit_sha): +def record_agent_profiles(conn, current_git_commit_sha, roles_dir=None): """Upsert the agent_profile (01-M 1.9) singleton rows for graphlemon's own repo.""" - for org_node in ORG_NODES: - agents_md_path = _agents_md_path(org_node) - agents_file_path = os.path.relpath(agents_md_path, _REPO_ROOT) if os.path.exists(agents_md_path) else None + for org_node in _discover_role_ids(roles_dir): + role = _load_role(org_node, roles_dir) + identity = role["identity"] + agents_md_path = identity.get("agents_md_path") conn.execute( "INSERT INTO agent_profile " "(org_node, soul_file_path, agents_file_path, migrated_from_profile, current_git_commit_sha) " @@ -163,9 +150,9 @@ def record_agent_profiles(conn, current_git_commit_sha): "current_git_commit_sha=excluded.current_git_commit_sha", ( org_node, - os.path.relpath(_soul_path(org_node), _REPO_ROOT), - agents_file_path, - _MIGRATED_FROM_PROFILE[org_node], + identity["soul_md_path"], + agents_md_path, + role["migrated_from_profile"], current_git_commit_sha, ), ) diff --git a/src/schema.py b/src/schema.py index 3ca156f..1cc0ed1 100644 --- a/src/schema.py +++ b/src/schema.py @@ -80,7 +80,12 @@ CREATE TABLE org_graph_definition ( -- references it) CREATE TABLE skill ( skill_id TEXT PRIMARY KEY, - owner_org_node TEXT NOT NULL CHECK (owner_org_node IN ('planner', 'coder', 'validator')), + -- H (2026-08-06, Role Package abstraction): was CHECK(owner_org_node IN + -- ('planner','coder','validator')) -- relaxed to a plain non-empty-string + -- check so a new role, added purely via roles//role.json (no + -- schema.py/agent_profile.py code change), isn't rejected by a stale + -- hardcoded enum. See docs/todo/role-package-abstraction-design.md §4. + owner_org_node TEXT NOT NULL CHECK (owner_org_node != ''), file_path TEXT NOT NULL, current_git_commit_sha TEXT NOT NULL, source TEXT NOT NULL CHECK (source IN ('migrated_from_hermes', 'new', 'auto_iterated')), @@ -105,7 +110,8 @@ CREATE TABLE task ( route TEXT CHECK (route IN ('fast_lane', 'full_replan')), loop_round INTEGER NOT NULL DEFAULT 0, blocked_reason TEXT, - assigned_org_node TEXT NOT NULL CHECK (assigned_org_node IN ('planner', 'coder', 'validator')), + -- H (2026-08-06): see skill.owner_org_node's comment above -- same relaxation. + assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP ); @@ -118,7 +124,8 @@ CREATE INDEX idx_task_project_id ON task(project_id); CREATE TABLE work_graph_run ( run_id TEXT PRIMARY KEY, task_id TEXT NOT NULL REFERENCES task(task_id), - org_node TEXT NOT NULL CHECK (org_node IN ('planner', 'coder', 'validator')), + -- H (2026-08-06): see skill.owner_org_node's comment above -- same relaxation. + org_node TEXT NOT NULL CHECK (org_node != ''), graph_manifest TEXT NOT NULL, orchestration_script_ref TEXT, status TEXT NOT NULL CHECK (status IN ( @@ -217,7 +224,8 @@ CREATE TABLE skill_iteration ( -- 1.9 agent_profile CREATE TABLE agent_profile ( - org_node TEXT PRIMARY KEY CHECK (org_node IN ('planner', 'coder', 'validator')), + -- H (2026-08-06): see skill.owner_org_node's comment above -- same relaxation. + org_node TEXT PRIMARY KEY CHECK (org_node != ''), soul_file_path TEXT NOT NULL, agents_file_path TEXT, migrated_from_profile TEXT NOT NULL CHECK (migrated_from_profile IN ('planning', 'code', 'qa')), @@ -426,3 +434,150 @@ def migrate_add_work_graph_fork_columns(conn): "REFERENCES work_node_execution(node_exec_id)" ) conn.commit() + + +def _table_sql(conn, table_name): + row = conn.execute( + "SELECT sql FROM sqlite_master WHERE type='table' AND name=?", (table_name,) + ).fetchone() + return row[0] if row else None + + +def migrate_relax_org_node_constraints(conn): + """H (2026-08-06, Role Package abstraction): relaxes the 4 tables whose + org_node-ish column was CHECK-constrained to exactly ('planner', + 'coder', 'validator') down to CHECK(x != '') -- a new role, added + purely via roles//role.json (see src/agent_profile.py), no + longer gets rejected by a schema-level enum baked in at U-001. + + **Why all 4 need the create-copy-drop-rename dance, not a plain ALTER + TABLE**: SQLite has no `ALTER TABLE ... DROP CONSTRAINT` for CHECK. + `skill`/`task`/`work_graph_run` are referenced by several other tables' + foreign keys (work_node_execution, artifact, bug_record, + execution_event, work_graph_run's own self-reference) -- but since only + the PARENT table's own CHECK text changes (the FK column definitions + in those child tables, e.g. `REFERENCES task(task_id)`, don't change at + all), only these 4 parent tables need rebuilding, none of their + children. Followed the SQLite-documented procedure for schema changes + under active foreign keys ("Making Other Kinds Of Table Schema + Changes"): PRAGMA foreign_keys=OFF for the duration (FK enforcement + can't be toggled mid-transaction), rebuild, PRAGMA foreign_key_check to + confirm nothing went dangling, PRAGMA foreign_keys=ON again. + + Depends on migrate_add_work_graph_fork_columns() having already run + (called here unconditionally, it's idempotent) -- work_graph_run's + rebuild needs to know about D's parent_run_id/fork_of_node_exec_id + columns whether or not this specific database has seen that migration + yet. + + Idempotent: each table's rebuild is skipped if its stored CREATE TABLE + text no longer contains the old enum CHECK, so a fresh + create_schema()'d database (already on the new constraint) is a no-op, + same as migrate_expand_artifact_types's own convention. + """ + migrate_add_work_graph_fork_columns(conn) + conn.commit() + + conn.execute("PRAGMA foreign_keys = OFF") + try: + if "owner_org_node IN (" in (_table_sql(conn, "skill") or ""): + conn.executescript( + """ + CREATE TABLE skill_new ( + skill_id TEXT PRIMARY KEY, + owner_org_node TEXT NOT NULL CHECK (owner_org_node != ''), + file_path TEXT NOT NULL, + current_git_commit_sha TEXT NOT NULL, + source TEXT NOT NULL CHECK (source IN ('migrated_from_hermes', 'new', 'auto_iterated')), + migrated_from_profile TEXT CHECK (migrated_from_profile IN ('code', 'planning', 'qa')), + status TEXT NOT NULL CHECK (status IN ('active', 'archived')) + ); + INSERT INTO skill_new SELECT * FROM skill; + DROP TABLE skill; + ALTER TABLE skill_new RENAME TO skill; + """ + ) + + if "assigned_org_node IN (" in (_table_sql(conn, "task") or ""): + conn.executescript( + """ + CREATE TABLE task_new ( + task_id TEXT PRIMARY KEY, + project_id TEXT NOT NULL REFERENCES project(project_id), + goal_text TEXT NOT NULL, + task_type TEXT NOT NULL CHECK (task_type IN ('goal', 'bug_fix')), + origin_ref TEXT, + created_via TEXT NOT NULL DEFAULT 'api' CHECK (created_via IN ('api', 'human_console')), + status TEXT NOT NULL CHECK (status IN ( + 'queued', 'planning_brd_prd_sdd', 'planning_failed', + 'coding', 'coding_failed', 'validating', + 'routed_fast_lane', 'routed_full_replan', + 'blocked_pending_human', 'done' + )), + route TEXT CHECK (route IN ('fast_lane', 'full_replan')), + loop_round INTEGER NOT NULL DEFAULT 0, + blocked_reason TEXT, + assigned_org_node TEXT NOT NULL CHECK (assigned_org_node != ''), + created_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP, + updated_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP + ); + INSERT INTO task_new SELECT * FROM task; + DROP TABLE task; + ALTER TABLE task_new RENAME TO task; + CREATE INDEX idx_task_status ON task(status); + CREATE INDEX idx_task_assigned_org_node ON task(assigned_org_node); + CREATE INDEX idx_task_project_id ON task(project_id); + """ + ) + + if "org_node IN (" in (_table_sql(conn, "work_graph_run") or ""): + conn.executescript( + """ + CREATE TABLE work_graph_run_new ( + run_id TEXT PRIMARY KEY, + task_id TEXT NOT NULL REFERENCES task(task_id), + org_node TEXT NOT NULL CHECK (org_node != ''), + graph_manifest TEXT NOT NULL, + orchestration_script_ref TEXT, + status TEXT NOT NULL CHECK (status IN ( + 'planning', 'running', 'merging', 'verifying', + 'reflecting', 'blocked', 'done', 'failed' + )), + started_at TEXT, + completed_at TEXT, + result_artifact_refs TEXT, + parent_run_id TEXT REFERENCES work_graph_run(run_id), + fork_of_node_exec_id TEXT REFERENCES work_node_execution(node_exec_id) + ); + INSERT INTO work_graph_run_new SELECT * FROM work_graph_run; + DROP TABLE work_graph_run; + ALTER TABLE work_graph_run_new RENAME TO work_graph_run; + CREATE UNIQUE INDEX ux_work_graph_run_active ON work_graph_run(task_id, org_node) + WHERE status IN ('planning', 'running', 'merging', 'verifying'); + """ + ) + + if "org_node TEXT PRIMARY KEY CHECK (org_node IN (" in (_table_sql(conn, "agent_profile") or ""): + conn.executescript( + """ + CREATE TABLE agent_profile_new ( + org_node TEXT PRIMARY KEY CHECK (org_node != ''), + soul_file_path TEXT NOT NULL, + agents_file_path TEXT, + migrated_from_profile TEXT NOT NULL CHECK (migrated_from_profile IN ('planning', 'code', 'qa')), + current_git_commit_sha TEXT NOT NULL + ); + INSERT INTO agent_profile_new SELECT * FROM agent_profile; + DROP TABLE agent_profile; + ALTER TABLE agent_profile_new RENAME TO agent_profile; + """ + ) + + conn.commit() + violations = conn.execute("PRAGMA foreign_key_check").fetchall() + if violations: + raise RuntimeError( + "migrate_relax_org_node_constraints left dangling foreign keys: {}".format(violations) + ) + finally: + conn.execute("PRAGMA foreign_keys = ON") diff --git a/src/test_agent_profile.py b/src/test_agent_profile.py index 0846ced..9608d2d 100644 --- a/src/test_agent_profile.py +++ b/src/test_agent_profile.py @@ -5,6 +5,7 @@ src/test_agent_e2e.py (RUN_REAL_AGENT_E2E=1) for the actual A-2 validation. Run: python3 -m unittest src.test_agent_profile -v """ +import json import os import tempfile import unittest @@ -71,6 +72,70 @@ class RenderAgentDefinitionTest(unittest.TestCase): self.assertNotIn("优先使用 claude --dangerously-skip-permissions", content) +class NewRoleWithoutCodeChangeTest(unittest.TestCase): + """H (2026-08-06), requirements doc acceptance standard 1: a brand-new + role, defined purely by adding a roles//role.json, is + recognized by these UNMODIFIED functions (render_agent_definition/ + deploy_agent_definitions/record_agent_profiles) via the roles_dir + override -- no agent_profile.py/schema.py edit involved. Points + identity.soul_md_path at a real, already-existing repo file + (agents/planner/SOUL.md) rather than fabricating a new one -- this + standard is about the roles/ discovery + rendering mechanism, not + about proving a new identity file can be written (unrelated concern).""" + + def setUp(self): + self.tmp = tempfile.TemporaryDirectory() + self.addCleanup(self.tmp.cleanup) + role_dir = os.path.join(self.tmp.name, "researcher") + os.makedirs(role_dir) + role = { + "role_id": "researcher", + "description": "A brand-new role added with zero Python/schema changes.", + "migrated_from_profile": "planning", + "identity": {"soul_md_path": "agents/planner/SOUL.md", "agents_md_path": None}, + "context": {"memory": "project"}, + "model": "claude-sonnet-5", + "skills": ["planning-requirement-expansion"], + "applications": {"mcp_servers": []}, + "workflows": {"dynamic_planning": "generic"}, + "permissions": {"tools": ["Read", "Grep"], "disallowed_tools": [], "notes": []}, + "acceptance_criteria": {"ref": "docs/sdd/06-验收与测试锚点.md"}, + } + with open(os.path.join(role_dir, "role.json"), "w", encoding="utf-8") as f: + json.dump(role, f) + + def test_discovered_alongside_the_real_three_roles(self): + discovered = agent_profile._discover_role_ids(self.tmp.name) + self.assertIn("researcher", discovered) + + def test_render_agent_definition_recognizes_it(self): + content = agent_profile.render_agent_definition("researcher", roles_dir=self.tmp.name) + self.assertIn("name: researcher", content) + self.assertIn("- planning-requirement-expansion", content) + + def test_deploy_agent_definitions_writes_it_out(self): + with tempfile.TemporaryDirectory() as target: + agent_profile.deploy_agent_definitions(target, roles_dir=self.tmp.name) + path = os.path.join(target, ".claude", "agents", "researcher.md") + self.assertTrue(os.path.isfile(path)) + + def test_record_agent_profiles_writes_a_row(self): + conn = schema.connect(":memory:") + try: + schema.create_schema(conn) + agent_profile.record_agent_profiles(conn, "deadbeef", roles_dir=self.tmp.name) + row = conn.execute( + "SELECT migrated_from_profile FROM agent_profile WHERE org_node = 'researcher'" + ).fetchone() + self.assertEqual(row, ("planning",)) + finally: + conn.close() + + def test_unknown_role_still_rejected(self): + with self.assertRaises(ValueError): + agent_profile.render_agent_definition("no-such-role", roles_dir=self.tmp.name) + + class DeployAgentDefinitionsTest(unittest.TestCase): def test_writes_three_files_into_target_claude_agents(self): with tempfile.TemporaryDirectory() as tmp: diff --git a/src/test_schema.py b/src/test_schema.py index 9fa7994..7143b3b 100644 --- a/src/test_schema.py +++ b/src/test_schema.py @@ -550,5 +550,192 @@ class MigrateAddWorkGraphForkColumnsTest(unittest.TestCase): fresh.close() +def _old_org_node_enum_schema_sql(): + """H (2026-08-06): simulates a pre-migration database whose 4 + org_node-ish columns still have the old CHECK(x IN ('planner','coder', + 'validator')) enum, and whose work_graph_run predates D's fork columns + too (migrate_relax_org_node_constraints depends on + migrate_add_work_graph_fork_columns having run, so the test fixture + should start from a database that's had NEITHER migration, same as a + real old production db would be).""" + sql = schema.SCHEMA_SQL + for column in ("owner_org_node", "assigned_org_node", "org_node"): + sql = re.sub( + r"(?: --[^\n]*\n)* " + column + r" TEXT( NOT NULL)? CHECK \(" + column + r" != ''\)", + column + r" TEXT\1 CHECK (" + column + " IN ('planner', 'coder', 'validator'))", + sql, + ) + sql = re.sub( + r"(?: --[^\n]*\n)* org_node TEXT PRIMARY KEY CHECK \(org_node != ''\)", + "org_node TEXT PRIMARY KEY CHECK (org_node IN ('planner', 'coder', 'validator'))", + sql, + ) + sql = re.sub( + r",\n( --[^\n]*\n)* parent_run_id TEXT REFERENCES work_graph_run\(run_id\),\n" + r" fork_of_node_exec_id TEXT REFERENCES work_node_execution\(node_exec_id\)\n\);", + "\n);", + sql, + ) + return sql + + +class MigrateRelaxOrgNodeConstraintsTest(unittest.TestCase): + """H (2026-08-06, Role Package abstraction): real upgrade path for an + already-populated database whose skill/task/work_graph_run/ + agent_profile tables still enum-CHECK org_node to exactly + ('planner', 'coder', 'validator').""" + + def setUp(self): + old_sql = _old_org_node_enum_schema_sql() + self.assertNotIn("!= ''", old_sql) # sanity: the substitution actually matched + self.assertNotIn("parent_run_id", old_sql) + + self.conn = schema.connect(":memory:") + self.conn.execute("PRAGMA foreign_keys = ON") + self.conn.executescript(old_sql) + self.conn.commit() + self.conn.execute( + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" + ) + self.conn.execute( + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " + "VALUES ('task-1', 'proj-1', 'g', 'goal', 'queued', 'planner')" + ) + self.conn.execute( + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " + "VALUES ('run-1', 'task-1', 'planner', '{}', 'done')" + ) + self.conn.execute( + "INSERT INTO skill (skill_id, owner_org_node, file_path, current_git_commit_sha, source, status) " + "VALUES ('skill-1', 'planner', 'skills/x/', 'sha', 'new', 'active')" + ) + self.conn.execute( + "INSERT INTO agent_profile (org_node, soul_file_path, migrated_from_profile, current_git_commit_sha) " + "VALUES ('planner', 'agents/planner/SOUL.md', 'planning', 'sha')" + ) + self.conn.commit() + + def tearDown(self): + self.conn.close() + + def test_old_schema_rejects_a_new_role(self): + with self.assertRaises(sqlite3.IntegrityError): + self.conn.execute( + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " + "VALUES ('task-2', 'proj-1', 'g', 'goal', 'queued', 'newrole')" + ) + + def test_migration_preserves_existing_rows(self): + schema.migrate_relax_org_node_constraints(self.conn) + self.assertEqual( + self.conn.execute("SELECT task_id, assigned_org_node FROM task WHERE task_id = 'task-1'").fetchone(), + ("task-1", "planner"), + ) + self.assertEqual( + self.conn.execute("SELECT skill_id, owner_org_node FROM skill WHERE skill_id = 'skill-1'").fetchone(), + ("skill-1", "planner"), + ) + self.assertEqual( + self.conn.execute("SELECT run_id, org_node FROM work_graph_run WHERE run_id = 'run-1'").fetchone(), + ("run-1", "planner"), + ) + self.assertEqual( + self.conn.execute("SELECT org_node FROM agent_profile WHERE org_node = 'planner'").fetchone(), + ("planner",), + ) + + def test_migration_accepts_a_new_role_afterward(self): + schema.migrate_relax_org_node_constraints(self.conn) + self.conn.execute( + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " + "VALUES ('task-2', 'proj-1', 'g', 'goal', 'queued', 'newrole')" + ) + self.conn.execute( + "INSERT INTO skill (skill_id, owner_org_node, file_path, current_git_commit_sha, source, status) " + "VALUES ('skill-2', 'newrole', 'skills/y/', 'sha', 'new', 'active')" + ) + self.conn.execute( + "INSERT INTO agent_profile (org_node, soul_file_path, migrated_from_profile, current_git_commit_sha) " + "VALUES ('newrole', 'roles/newrole/SOUL.md', 'planning', 'sha')" + ) + self.conn.commit() + self.assertEqual(self.conn.execute("SELECT COUNT(*) FROM task").fetchone()[0], 2) + + def test_migration_still_rejects_empty_string(self): + schema.migrate_relax_org_node_constraints(self.conn) + with self.assertRaises(sqlite3.IntegrityError): + self.conn.execute( + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " + "VALUES ('task-2', 'proj-1', 'g', 'goal', 'queued', '')" + ) + + def test_ux_work_graph_run_active_still_enforced_after_rebuild(self): + schema.migrate_relax_org_node_constraints(self.conn) + self.conn.execute( + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " + "VALUES ('run-2', 'task-1', 'coder', '{}', 'running')" + ) + with self.assertRaises(sqlite3.IntegrityError): + self.conn.execute( + "INSERT INTO work_graph_run (run_id, task_id, org_node, graph_manifest, status) " + "VALUES ('run-3', 'task-1', 'coder', '{}', 'running')" + ) + + def test_task_indexes_survive_rebuild(self): + schema.migrate_relax_org_node_constraints(self.conn) + index_names = { + row[1] for row in self.conn.execute("PRAGMA index_list(task)").fetchall() + } + self.assertIn("idx_task_status", index_names) + self.assertIn("idx_task_assigned_org_node", index_names) + self.assertIn("idx_task_project_id", index_names) + + def test_foreign_key_relationships_survive_rebuild(self): + # work_node_execution references both skill and work_graph_run -- + # confirms the child tables (never rebuilt themselves) still see a + # valid parent after skill/work_graph_run were dropped and recreated. + schema.migrate_relax_org_node_constraints(self.conn) + self.conn.execute( + "INSERT INTO work_node_execution (node_exec_id, run_id, node_id, subagent_type, skill_ref, " + "input_schema_ref, output_schema_ref, input_payload, status) " + "VALUES ('node-1', 'run-1', 'n1', 'planner', 'skill-1', 'in.json', 'out.json', '{}', 'done')" + ) + self.conn.commit() + with self.assertRaises(sqlite3.IntegrityError): + self.conn.execute( + "INSERT INTO work_node_execution (node_exec_id, run_id, node_id, subagent_type, skill_ref, " + "input_schema_ref, output_schema_ref, input_payload, status) " + "VALUES ('node-2', 'no-such-run', 'n1', 'planner', 'skill-1', 'in.json', 'out.json', '{}', 'done')" + ) + + def test_idempotent_on_a_database_already_migrated(self): + schema.migrate_relax_org_node_constraints(self.conn) + schema.migrate_relax_org_node_constraints(self.conn) # must not raise + count = self.conn.execute("SELECT COUNT(*) FROM task").fetchone()[0] + self.assertEqual(count, 1) + + def test_foreign_keys_pragma_restored_after_migration(self): + schema.migrate_relax_org_node_constraints(self.conn) + self.assertEqual(self.conn.execute("PRAGMA foreign_keys").fetchone()[0], 1) + + def test_noop_on_a_freshly_created_already_relaxed_database(self): + fresh = schema.connect(":memory:") + try: + schema.create_schema(fresh) + schema.migrate_relax_org_node_constraints(fresh) # must not raise + fresh.execute( + "INSERT INTO project (project_id, name, target_folder_path, has_git_repo, status) " + "VALUES ('proj-1', 'P', '/tmp/proj-1', 1, 'active')" + ) + fresh.execute( + "INSERT INTO task (task_id, project_id, goal_text, task_type, status, assigned_org_node) " + "VALUES ('task-1', 'proj-1', 'g', 'goal', 'queued', 'newrole')" + ) + fresh.commit() + finally: + fresh.close() + + if __name__ == "__main__": unittest.main() diff --git a/src/work_graph_run_query.py b/src/work_graph_run_query.py index 24b1561..824464b 100644 --- a/src/work_graph_run_query.py +++ b/src/work_graph_run_query.py @@ -22,7 +22,14 @@ helper is private to its own module and this module already owns import json -_VALID_ORG_NODES = ("planner", "coder", "validator") +from src import agent_profile + +# H (2026-08-06): was its own hardcoded ('planner', 'coder', 'validator') +# tuple -- a real dependency the requirements doc's own risk analysis +# missed (see docs/todo/graph-engineering-open-items.md's H review note). +# Now reuses agent_profile.ORG_NODES so this validation stays data-driven +# too, not a second place someone has to remember to update for a new role. +_VALID_ORG_NODES = agent_profile.ORG_NODES class InvalidParamError(Exception):