Commit 78be2e6fd37dd0af6faf97d77e771020418606ff
1 parent
c788bbac
graphlemon: sdd artifact for task b243dfe8-4da8-49bd-895b-8b9e9ed17280
Showing
1 changed file
with
408 additions
and
0 deletions
.graphlemon/runs/b243dfe8-4da8-49bd-895b-8b9e9ed17280/sdd.md
0 → 100644
| 1 | +# SDD:Skill 详情页展示真实内容(`v_skill_audit_log` 契约收口) | ||
| 2 | + | ||
| 3 | +状态:Approved PRD 已定稿,本 SDD 据其落地设计。 | ||
| 4 | +对应 PRD:《PRD:Skill 详情页展示真实内容》(本任务 goal 的同批交付物)。 | ||
| 5 | +涉及模块:`src/skill_query.py`、`src/skill_update.py`(只读引用,不修改其写路径逻辑)、`src/backend_server.py`、`frontend/app.js`、对应测试文件。 | ||
| 6 | + | ||
| 7 | +--- | ||
| 8 | + | ||
| 9 | +## 0. 前置说明:PRD 根因表述与当前代码基线的差异(必读,决定本 Unit 的真实范围) | ||
| 10 | + | ||
| 11 | +PRD 背景部分描述的根因是:“`src/skill_query.py` 目前只有 `list_skills()` 和 | ||
| 12 | +`get_skill_audit_log()`,没有任何函数读取 `skills/<skill_id>/SKILL.md` 的真实内容”。 | ||
| 13 | + | ||
| 14 | +**经代码核实(2026-08-13),这个前提已不成立**:当前 `src/skill_query.py` 已经存在 | ||
| 15 | +`get_skill_content()`(含 `SkillContentMissingError`、两层路径包含性校验、`_skill_file_path` | ||
| 16 | +复用),`src/backend_server.py` 已经存在 `GET /api/skills/:skill_id/content` 路由和 | ||
| 17 | +`_handle_skill_content`,`frontend/app.js` 的 `renderSkillAuditLog`/`mountSkillAuditLog` 也已经 | ||
| 18 | +在渲染一个独立的“Skill 内容”面板,`src/test_skill_query.py`(`GetSkillContentTest` 一类用例)和 | ||
| 19 | +`src/test_backend_server.py::SkillRouteTest`(`test_content_*` 系列)也都已经覆盖了这套实现。 | ||
| 20 | + | ||
| 21 | +换句话说:**“页面点击 Skill 不显示真实内容”这个用户可见的 bug,字面意义上可能已经不存在**—— | ||
| 22 | +打开 `#/skills/:skill_id/audit` 今天就能看到“Skill 内容”面板和真实文本。但本 PRD 在“背景”之外 | ||
| 23 | +还在 §4.1/§4.2/FR-1/FR-2/FR-3 里做出了一组具体的**设计决策**,而这些决策与当前实现在契约形状上 | ||
| 24 | +**不一致**: | ||
| 25 | + | ||
| 26 | +| 维度 | 当前代码实际行为 | PRD 决策要求的行为 | | ||
| 27 | +|---|---|---| | ||
| 28 | +| 路由形态 | `GET /api/skills/:skill_id/content`(`/content` 后缀) | `GET /api/skills/:skill_id`(与 `/audit` 并列的裸资源路由,§4.1) | | ||
| 29 | +| 支持文件字段名 | `support_files` | `supporting_files`(§4.1 JSON 示例、FR-1) | | ||
| 30 | +| 响应体字段完整性 | 仅 `{skill_id, content, support_files}` | 还需包含 `owner_org_node`/`file_path`/`current_git_commit_sha`/`source`/`migrated_from_profile`/`status`(§4.1) | | ||
| 31 | +| 磁盘文件缺失时的语义 | 抛 `SkillContentMissingError` → 404 `CONTENT_MISSING` | 200,`content: null`,不抛异常(FR-1、§6) | | ||
| 32 | +| 路径穿越(`file_path` 越界)时的语义 | 同样抛 `SkillContentMissingError` → 404 `CONTENT_MISSING`(与“文件缺失”共用同一个异常/错误码,前端无法区分) | 专门的 `SkillPathTraversalError` → 400 `INVALID_SKILL_ID`(§4.1、FR-2、§6 安全表) | | ||
| 33 | +| skill 不存在 vs 内容缺失的页面态 | “Skill 内容”面板独立显示提示文案,“变更审计”面板独立显示 404 横幅;skill 完全不存在时两个面板各自处理,互不联动 | skill 完全不存在时两个面板共用同一条顶部错误横幅、都不渲染(FR-3、AC5) | | ||
| 34 | + | ||
| 35 | +**本 Unit 的真实工作内容因此是“契约收口”,不是“从零实现”**:复用已经写好的读取/展示骨架, | ||
| 36 | +把上表右列的决策真正落地,同时保留骨架里仍然正确、PRD 未要求改变的部分(`_skill_file_path` | ||
| 37 | +复用、`realpath`+前缀包含性校验的整体思路、`skill_iteration` 面板完全不动)。 | ||
| 38 | + | ||
| 39 | +PRD §7 写道“现有 `src/test_skill_query.py`、`src/test_backend_server.py::SkillRouteTest` 中的既 | ||
| 40 | +有用例必须全部保持通过,不允许为了实现新功能而修改其断言”——这句话是在“该函数尚不存在”这个 | ||
| 41 | +(已不成立的)前提下写的,字面执行会与上表要求的契约变更直接冲突(例如 `support_files` 必须 | ||
| 42 | +改名为 `supporting_files`,测试断言不可能不变)。本 SDD 对此的裁决(决策,非独走): | ||
| 43 | + | ||
| 44 | +> **§7 的“既有用例不可修改”原则,适用范围收窄为“不测试 `get_skill_content`/`/content` | ||
| 45 | +> 路由本身契约形状”的用例**(即 `test_lists_all_skills`、`test_skill_with_no_iterations_returns | ||
| 46 | +> _empty_list`、`test_full_update_diff_rollback_cycle`、`test_rollback_*`、`test_diff_*`、 | ||
| 47 | +> `test_audit_log_*` 等——这些测试 `list_skills`/`get_skill_audit_log`/`update_skill`/ | ||
| 48 | +> `rollback_skill`/`get_diff`,与本次契约变更无关,必须保持通过不动)。 | ||
| 49 | +> 而 `GetSkillContentTest` 全部用例、`SkillRouteTest.test_content_*` 三个用例,测试的正是本次要 | ||
| 50 | +> 改变契约形状的那个函数/路由本身,**必须随契约一起更新**——这不是“为了让新功能过测试而放宽 | ||
| 51 | +> 断言”,是给同一段代码换了一份新的、PRD 已批准的验收契约。Code Agent 落地时应把这几个测试方 | ||
| 52 | +> 法**重写为断言新契约**,而不是删除或跳过。 | ||
| 53 | + | ||
| 54 | +--- | ||
| 55 | + | ||
| 56 | +## 1. 架构映射(M/V/C,遵循仓库既有 MVCP 记法) | ||
| 57 | + | ||
| 58 | +- **M(数据)**:不变。`skill` 表 schema 不动;磁盘上 `skills/<skill_id>/SKILL.md` 及可能的 | ||
| 59 | + 支持文件是“数据”的另一半(platform asset,`skill_query.py` 顶部注释已有此定位),本次只读。 | ||
| 60 | +- **C(控制器/函数)**: | ||
| 61 | + - `src/skill_query.py::get_skill_content()` —— 改造对象,函数名不变,签名`(conn, skill_id, | ||
| 62 | + repo_path=None)` 不变,返回值形状和异常类型变。 | ||
| 63 | + - 新增 `src/skill_query.py::SkillPathTraversalError`。 | ||
| 64 | + - `src/backend_server.py::_handle_skill_content` —— 改造对象;路由注册从 `/content` 后缀改为 | ||
| 65 | + 裸 `skill_id` 分支,函数名可保留(或按第 3.2 节改名 `_handle_skill_get`,由实现选择,只要 | ||
| 66 | + 路由行为符合 §3.2 的匹配顺序)。 | ||
| 67 | + - `src/skill_update.py::_skill_file_path` —— 只读复用,不修改。 | ||
| 68 | +- **V(视图)**:`frontend/app.js::renderSkillAuditLog`/`mountSkillAuditLog` —— 改造对象,DOM | ||
| 69 | + 结构基本保留(“Skill 内容”面板已经存在,位置、标题不变),改的是 fetch 的 URL、字段名、以及 | ||
| 70 | + 错误态/`content===null` 态的呈现逻辑。对应 wireframe:`docs/wireframes/v_skill_audit_log.html` | ||
| 71 | + (如该 wireframe 里的字段名/结构与新契约不一致,一并同步,属于本 Unit 的收尾项,不需要单独 | ||
| 72 | + 开 Unit)。 | ||
| 73 | + | ||
| 74 | +--- | ||
| 75 | + | ||
| 76 | +## 2. 目标契约设计 | ||
| 77 | + | ||
| 78 | +### 2.1 `GET /api/skills/:skill_id` 响应契约(替代旧 `GET /api/skills/:skill_id/content`) | ||
| 79 | + | ||
| 80 | +请求:`GET /api/skills/skill-1` | ||
| 81 | + | ||
| 82 | +成功(200): | ||
| 83 | + | ||
| 84 | +```json | ||
| 85 | +{ | ||
| 86 | + "skill_id": "skill-1", | ||
| 87 | + "owner_org_node": "coder", | ||
| 88 | + "file_path": "skills/skill-1", | ||
| 89 | + "current_git_commit_sha": "unknown", | ||
| 90 | + "source": "migrated_from_hermes", | ||
| 91 | + "migrated_from_profile": "code", | ||
| 92 | + "status": "active", | ||
| 93 | + "content": "...SKILL.md 全文,UTF-8;文件缺失时为 null...", | ||
| 94 | + "supporting_files": ["references/foo.md", "scripts/bar.py"] | ||
| 95 | +} | ||
| 96 | +``` | ||
| 97 | + | ||
| 98 | +错误: | ||
| 99 | + | ||
| 100 | +| 情形 | HTTP | body | | ||
| 101 | +|---|---|---| | ||
| 102 | +| `skill_id` 在 `skill` 表中不存在 | 404 | `{"error": "NOT_FOUND"}` | | ||
| 103 | +| `skill.file_path` 解析出的绝对路径落在 `repo_root/skills/` 之外(无论是历史数据脏值还是未来 | ||
| 104 | + 被篡改) | 400 | `{"error": "INVALID_SKILL_ID"}` | | ||
| 105 | +| 上述两种情形都不成立,但磁盘 `SKILL.md` 不存在(DB 有行,文件缺失/尚未迁移) | **200**,`content: null` | 不是错误 | | ||
| 106 | + | ||
| 107 | +旧路由 `GET /api/skills/:skill_id/content` 直接移除(不保留兼容别名——PRD 明确是“新增一个兄弟 | ||
| 108 | +接口”而非“新增一个后缀”,且当前唯一调用方就是本仓库自己的前端,随本 Unit 一并切换,没有第三方 | ||
| 109 | +消费者需要兼容期)。 | ||
| 110 | + | ||
| 111 | +### 2.2 路由匹配顺序(`src/backend_server.py::do_GET`) | ||
| 112 | + | ||
| 113 | +在既有 `_SKILLS_PREFIX` 相关分支中,新分支必须放在 `/audit` 分支**之后**、旧 `/content` 分支 | ||
| 114 | +**原地替换**: | ||
| 115 | + | ||
| 116 | +```python | ||
| 117 | +if parsed.path == "/api/skills": | ||
| 118 | + self._handle_skill_list(); return | ||
| 119 | +if parsed.path == "/api/skills/diff": | ||
| 120 | + self._handle_skill_diff(parsed.query); return | ||
| 121 | +if parsed.path.startswith(_SKILLS_PREFIX) and parsed.path.endswith("/audit"): | ||
| 122 | + ...; self._handle_skill_audit_log(skill_id); return | ||
| 123 | +# 原来这里是 endswith("/content") 分支 -- 整段删除,替换为下面的裸 skill_id 分支 | ||
| 124 | +if parsed.path.startswith(_SKILLS_PREFIX): | ||
| 125 | + skill_id = parsed.path[len(_SKILLS_PREFIX):] | ||
| 126 | + if skill_id and "/" not in skill_id: | ||
| 127 | + self._handle_skill_get(skill_id) | ||
| 128 | + return | ||
| 129 | +``` | ||
| 130 | + | ||
| 131 | +这与 `_ARTIFACTS_PREFIX`(`GET /api/artifacts/:artifact_id`)已有的“剩余路径不含 `/` 才命中”写 | ||
| 132 | +法完全同构,不发明新的路由匹配范式。`POST .../rollback` 分支在 `do_POST` 里,`_SKILLS_PREFIX` | ||
| 133 | +复用不冲突,无需改动。 | ||
| 134 | + | ||
| 135 | +### 2.3 `get_skill_content()` 新契约 | ||
| 136 | + | ||
| 137 | +```python | ||
| 138 | +class SkillPathTraversalError(Exception): | ||
| 139 | + """INVALID_SKILL_ID -- skill.file_path 解析出的绝对路径落在 repo_root/skills/ | ||
| 140 | + 之外。永远在打开任何文件之前抛出(fail closed)。与 SkillNotFoundError | ||
| 141 | + (skill_id 在 DB 里就查不到)和"磁盘文件缺失但路径合法"(不再是异常,见 | ||
| 142 | + 下方 content=None)是三种互斥的独立情形。""" | ||
| 143 | + | ||
| 144 | + | ||
| 145 | +def get_skill_content(conn, skill_id, repo_path=None): | ||
| 146 | + row = conn.execute( | ||
| 147 | + "SELECT owner_org_node, file_path, current_git_commit_sha, source, " | ||
| 148 | + "migrated_from_profile, status FROM skill WHERE skill_id = ?", (skill_id,) | ||
| 149 | + ).fetchone() | ||
| 150 | + if row is None: | ||
| 151 | + raise SkillNotFoundError(skill_id) | ||
| 152 | + owner_org_node, file_path, sha, source, migrated_from_profile, status = row | ||
| 153 | + | ||
| 154 | + repo_root = repo_path or _REPO_ROOT | ||
| 155 | + skills_root = os.path.realpath(os.path.join(repo_root, "skills")) | ||
| 156 | + rel_path = skill_update._skill_file_path(file_path) | ||
| 157 | + abs_path = os.path.realpath(os.path.join(repo_root, rel_path)) | ||
| 158 | + if os.path.commonpath([abs_path, skills_root]) != skills_root: | ||
| 159 | + raise SkillPathTraversalError(skill_id) # 400, 未打开任何文件 | ||
| 160 | + | ||
| 161 | + try: | ||
| 162 | + with open(abs_path, "r", encoding="utf-8", errors="replace") as f: | ||
| 163 | + content = f.read() | ||
| 164 | + except OSError: | ||
| 165 | + content = None # 200, 不是异常 -- 与 v_artifact_browser 的 content:null 约定对齐 | ||
| 166 | + | ||
| 167 | + skill_dir = os.path.dirname(abs_path) | ||
| 168 | + supporting_files = [] | ||
| 169 | + for dirpath, _dirnames, filenames in os.walk(skill_dir): # 目录不存在时 os.walk 直接返回空,不报错 | ||
| 170 | + for name in filenames: | ||
| 171 | + full_path = os.path.realpath(os.path.join(dirpath, name)) | ||
| 172 | + if full_path == abs_path: | ||
| 173 | + continue | ||
| 174 | + supporting_files.append(os.path.relpath(full_path, skill_dir)) | ||
| 175 | + supporting_files.sort() | ||
| 176 | + | ||
| 177 | + return { | ||
| 178 | + "skill_id": skill_id, "owner_org_node": owner_org_node, "file_path": file_path, | ||
| 179 | + "current_git_commit_sha": sha, "source": source, "migrated_from_profile": migrated_from_profile, | ||
| 180 | + "status": status, "content": content, "supporting_files": supporting_files, | ||
| 181 | + } | ||
| 182 | +``` | ||
| 183 | + | ||
| 184 | +关键变化点(相对当前代码): | ||
| 185 | + | ||
| 186 | +1. `SELECT` 从只取 `file_path` 改为取完整 skill 行(新增 5 个列),字段直接拼进返回 dict——不 | ||
| 187 | + 走 `list_skills()` 二次查询,避免一次请求两次打库;这是“由实现选择”范围内的合理做法(PRD | ||
| 188 | + FR-2 允许在 `skill_query` 层或 backend handler 层拼装,此处选择前者,因为已经有 skill 行在 | ||
| 189 | + 手,没有理由再查一次)。 | ||
| 190 | +2. 路径包含性校验失败(layer 1):异常类型从 `SkillContentMissingError` 改为 | ||
| 191 | + `SkillPathTraversalError`,语义从“内容不可用”收紧为“非法输入拒绝”。 | ||
| 192 | +3. 文件打开失败(layer 2,`OSError`):不再 `raise`,改为 `content = None` 后继续往下执行(还 | ||
| 193 | + 要继续枚举 `supporting_files`,即使 `SKILL.md` 本身缺失,目录里仍可能有其他文件——虽然当前 | ||
| 194 | + 仓库场景下不会出现“文件缺失但目录有其他文件”的组合,但函数逻辑上不应该因为主文件缺失就跳过 | ||
| 195 | + 目录枚举)。 | ||
| 196 | +4. `SkillContentMissingError` 类整体删除(不再有任何调用点会抛出它)——删除时先用 | ||
| 197 | + `grep -rn SkillContentMissingError src/` 确认调用方只有 `backend_server.py` 一处并同步改掉, | ||
| 198 | + 避免留下引用不存在符号的 `except` 分支。 | ||
| 199 | + | ||
| 200 | +### 2.4 `_handle_skill_content` → 改为裸 skill_id 的 handler | ||
| 201 | + | ||
| 202 | +```python | ||
| 203 | +def _handle_skill_get(self, skill_id): | ||
| 204 | + """GET /api/skills/:skill_id -- Skill 详情:DB 行字段 + 真实 SKILL.md 内容 | ||
| 205 | + + 支持文件清单,与 /audit(迭代审计)并列,两者响应体互不影响。""" | ||
| 206 | + try: | ||
| 207 | + with self.server.conn_lock: | ||
| 208 | + result = skill_query.get_skill_content(self.server.conn, skill_id, repo_path=self.server.skill_repo_path) | ||
| 209 | + except skill_query.SkillNotFoundError: | ||
| 210 | + self._send_json(404, {"error": "NOT_FOUND"}) | ||
| 211 | + except skill_query.SkillPathTraversalError: | ||
| 212 | + self._send_json(400, {"error": "INVALID_SKILL_ID"}) | ||
| 213 | + else: | ||
| 214 | + self._send_json(200, result) | ||
| 215 | +``` | ||
| 216 | + | ||
| 217 | +--- | ||
| 218 | + | ||
| 219 | +## 3. 安全设计:路径穿越两层防御(沿用既有思路,收紧输出语义) | ||
| 220 | + | ||
| 221 | +防御结构本身(不变,继续沿用当前代码已经做对的部分): | ||
| 222 | + | ||
| 223 | +1. **输入面**:`skill_id` 永远只用于参数化 SQL `WHERE skill_id = ?`,从不直接拼接文件系统路 | ||
| 224 | + 径——这一点当前代码已经做到,本次不引入回退。 | ||
| 225 | +2. **路径来源**:文件路径来自可信的 `skill.file_path` DB 列 + `skill_update._skill_file_path()` | ||
| 226 | + 的既有解析规则(复用,不重写),不是来自 `skill_id` 本身。 | ||
| 227 | +3. **纵深校验**:`os.path.realpath` 双方都规范化后做 `os.path.commonpath` 前缀包含性判断——沿 | ||
| 228 | + 用当前代码已验证过(含 `skills-evil/` 前缀误判的测试用例)的写法,不换算法。 | ||
| 229 | + | ||
| 230 | +**唯一变化**:越界情形下的对外语义从“和文件缺失共用同一个 404”变成独立的 400 与独立异常类 | ||
| 231 | +型。测试层面新增/沿用(当前 `GetSkillContentTest` 里的 | ||
| 232 | +`test_traversal_via_directory_component_returns_content_missing`、 | ||
| 233 | +`test_traversal_via_relative_dotdot_in_file_path` | ||
| 234 | +这两个已存在的用例本身逻辑正确,只需把 `assertRaises(SkillContentMissingError)` 改成 | ||
| 235 | +`assertRaises(SkillPathTraversalError)`,测试意图和覆盖面完全不用重新设计)。PRD §6 要求的“插 | ||
| 236 | +入 `file_path` 被篡改为 `"../../etc/"` 的记录,断言拒绝读取而不是把上层目录内容读出来返回”这 | ||
| 237 | +条,当前测试已经用哨兵式的“断言抛异常”方式间接满足(异常发生在 `open()` 之前,物理上不可能读 | ||
| 238 | +出内容);如需更直白的证据,可以在越界目录里放一个内容各异的哨兵文件,断言返回值里完全不含该 | ||
| 239 | +文件内容/文件名——非强制,现有的“抛异常且从不 open”已经是充分证据。 | ||
| 240 | + | ||
| 241 | +--- | ||
| 242 | + | ||
| 243 | +## 4. 前端设计(`frontend/app.js`) | ||
| 244 | + | ||
| 245 | +`renderSkillAuditLog` 的 DOM 结构**不需要改**(“Skill 内容”面板、`#skill_content`、 | ||
| 246 | +`#skill_audit_error`、`#skill_audit_list` 都已经是 PRD 要求的样子)。改动全部在 | ||
| 247 | +`mountSkillAuditLog` 内部: | ||
| 248 | + | ||
| 249 | +```js | ||
| 250 | +function mountSkillAuditLog(skillId) { | ||
| 251 | + const listEl = document.getElementById("skill_audit_list"); | ||
| 252 | + const errorEl = document.getElementById("skill_audit_error"); | ||
| 253 | + const contentEl = document.getElementById("skill_content"); | ||
| 254 | + | ||
| 255 | + function showNotFound() { | ||
| 256 | + // FR-3: skill 完全不存在时,两个面板共用同一条顶部错误横幅,都不渲染自己的内容 | ||
| 257 | + contentEl.innerHTML = ""; | ||
| 258 | + listEl.innerHTML = ""; | ||
| 259 | + errorEl.textContent = "该 Skill 不存在"; | ||
| 260 | + errorEl.style.display = "block"; | ||
| 261 | + } | ||
| 262 | + | ||
| 263 | + fetch("/api/skills/" + encodeURIComponent(skillId)).then(async (resp) => { | ||
| 264 | + if (resp.status === 404) { showNotFound(); return; } | ||
| 265 | + if (resp.status !== 200) { | ||
| 266 | + // 400 INVALID_SKILL_ID 等:现实中不可达(skill_id 已先过 DB 校验), | ||
| 267 | + // 但必须优雅降级,不能让面板空白报错 | ||
| 268 | + contentEl.innerHTML = '<div class="hint">Skill 内容暂不可用</div>'; | ||
| 269 | + return; | ||
| 270 | + } | ||
| 271 | + const skill = await resp.json(); | ||
| 272 | + const supportHtml = skill.supporting_files.length | ||
| 273 | + ? '<div class="hint">支持文件:' + skill.supporting_files.map(escapeHtml).join("、") + "</div>" | ||
| 274 | + : '<div class="hint">无支持文件</div>'; | ||
| 275 | + contentEl.innerHTML = supportHtml | ||
| 276 | + + '<pre class="code">' + (skill.content == null ? "(内容文件不可用)" : escapeHtml(skill.content)) + "</pre>"; | ||
| 277 | + }); | ||
| 278 | + | ||
| 279 | + async function refresh() { | ||
| 280 | + const resp = await fetch("/api/skills/" + encodeURIComponent(skillId) + "/audit"); | ||
| 281 | + if (resp.status === 404) { showNotFound(); return; } | ||
| 282 | + const iterations = await resp.json(); | ||
| 283 | + if (iterations.length === 0) { | ||
| 284 | + listEl.innerHTML = '<div class="hint">该 Skill 尚无自主迭代记录</div>'; | ||
| 285 | + return; | ||
| 286 | + } | ||
| 287 | + // ...其余迭代记录渲染逻辑完全不变,原样保留... | ||
| 288 | + } | ||
| 289 | + refresh(); | ||
| 290 | +} | ||
| 291 | +``` | ||
| 292 | + | ||
| 293 | +要点: | ||
| 294 | + | ||
| 295 | +- 内容渲染 `skill.content == null ? "(内容文件不可用)" : escapeHtml(skill.content)` 与 | ||
| 296 | + `showPreview()` 里 `artifact.content == null ? "(文件不可用)" : escapeHtml(artifact.content)` | ||
| 297 | + 同构(措辞留用当前 Skill 面板已经在用的“(内容文件不可用)”,不必和 artifact 面板逐字一致, | ||
| 298 | + PRD 原文也是这个措辞)——继续不引入任何 Markdown/代码高亮库。 | ||
| 299 | +- `supporting_files` 字段名同步改名(原 `support_files`)。 | ||
| 300 | +- 两个 fetch(`/api/skills/:id` 和 `/api/skills/:id/audit`)保持并行发起、独立渲染,任一方失败 | ||
| 301 | + 不阻塞另一方——这一点当前代码已经是这样,不用改并行结构,只需让“skill 完全不存在”这一种情形 | ||
| 302 | + 从两个面板分别处理收敛成共用 `showNotFound()`。 | ||
| 303 | +- `docs/wireframes/v_skill_audit_log.html` 若存在与新字段名/新面板结构不一致之处,随手同步(低 | ||
| 304 | + 成本收尾项,不需要单独验收)。 | ||
| 305 | + | ||
| 306 | +--- | ||
| 307 | + | ||
| 308 | +## 5. 兼容性与回归范围 | ||
| 309 | + | ||
| 310 | +- `GET /api/skills`、`GET /api/skills/:skill_id/audit`、`GET /api/skills/diff`、 | ||
| 311 | + `POST /api/skills/:skill_id/rollback` 四个接口的路由匹配、响应契约、行为完全不变。 | ||
| 312 | +- `GET /api/skills/:skill_id/content` 直接移除(无兼容层,见 §2.1 理由)。 | ||
| 313 | +- `src/test_skill_query.py` 中 `ListSkillsTest`、`GetSkillAuditLogTest`、`GetDiffTest` 三个测试 | ||
| 314 | + 类原样保留;`GetSkillContentTest` 类里的用例按 §0 裁决重写为新契约的断言,覆盖面不变(正常 | ||
| 315 | + 内容、缺失文件、Skill 不存在、路径穿越两种触发方式、支持文件枚举)。 | ||
| 316 | +- `src/test_backend_server.py::SkillRouteTest` 中 `test_list_returns_seeded_skill`、 | ||
| 317 | + `test_audit_log_*`、`test_diff_*`、`test_full_update_diff_rollback_cycle`、`test_rollback_*` | ||
| 318 | + 原样保留;`test_content_*` 三个方法重写为新契约(URL 从 `/content` 改为裸 id,字段名改,缺失 | ||
| 319 | + 文件场景断言从 404 改为 200+`content:null`)。 | ||
| 320 | + | ||
| 321 | +--- | ||
| 322 | + | ||
| 323 | +## 6. Development Unit 分解 | ||
| 324 | + | ||
| 325 | +单一纵向 Unit,理由:三层改动(`skill_query.py` 契约 → `backend_server.py` 路由/handler → | ||
| 326 | +`app.js` 消费方)互为强依赖,响应体形状是同一个契约的三处镜像,拆成多个 Unit 只会制造“后端改 | ||
| 327 | +完前端还在用旧字段名”的中间不可验收态,不产生任何独立可交付价值。规模上(3 个源文件改动 + 2 | ||
| 328 | +个测试文件改动,均为已有文件的局部修改,无新文件)单个 Code Agent 一次连贯实现完全可以覆盖, | ||
| 329 | +符合“合格 Unit 能交给单个 Code Agent,在一次连贯实现与评审周期中完成”的标准。 | ||
| 330 | + | ||
| 331 | +### DU-01:Skill 详情内容契约收口(bug fix) | ||
| 332 | + | ||
| 333 | +**类型**:Bug 修复(纵向,覆盖 M 不变 / C 两处 / V 一处)。 | ||
| 334 | + | ||
| 335 | +**范围**: | ||
| 336 | +1. `src/skill_query.py`:`get_skill_content()` 按 §2.3 重写;新增 `SkillPathTraversalError`; | ||
| 337 | + 删除 `SkillContentMissingError`(先确认无其他调用方)。 | ||
| 338 | +2. `src/backend_server.py`:按 §2.2 调整 `do_GET` 路由匹配顺序(移除 `/content` 后缀分支,新增 | ||
| 339 | + 裸 `skill_id` 分支);`_handle_skill_content` 按 §2.4 重写为 `_handle_skill_get`(或保留原函 | ||
| 340 | + 数名,仅改内部逻辑与调用点——由实现选择,行为以 §2.1/§2.4 为准)。 | ||
| 341 | +3. `frontend/app.js`:`mountSkillAuditLog` 按 §4 重写;字段名 `support_files`→ | ||
| 342 | + `supporting_files`;skill 不存在时两个面板共用顶部横幅。 | ||
| 343 | +4. 测试:`src/test_skill_query.py::GetSkillContentTest`、 | ||
| 344 | + `src/test_backend_server.py::SkillRouteTest.test_content_*` 按 §0/§5 重写为新契约断言;新增 | ||
| 345 | + “磁盘文件缺失 → 200 + content:null”“路径穿越 → 400 + INVALID_SKILL_ID”两个显式路由层测试 | ||
| 346 | + (当前 `SkillRouteTest` 里没有覆盖越界场景的路由层用例,仅 `skill_query` 单元层有——PRD §8 | ||
| 347 | + 要求两层都要有覆盖,这是本 Unit 需要真正新增的测试,不是重写)。 | ||
| 348 | +5. 顺手项(非必须但成本极低,同一次改动一起做):核实并按需更新 | ||
| 349 | + `docs/wireframes/v_skill_audit_log.html` 中与新字段名不一致的位置。 | ||
| 350 | + | ||
| 351 | +**不在范围内**(继承 PRD 非目标):`skill_update.py`/迁移逻辑本身;`skill_iteration` 查询逻 | ||
| 352 | +辑;支持文件的内容预览;Markdown 渲染引入。 | ||
| 353 | + | ||
| 354 | +**依赖**:无对其他未完成 Unit 的依赖;三处改动内部顺序建议 `skill_query.py` → | ||
| 355 | +`backend_server.py` → `app.js`(每一步都可独立跑对应层的测试验证),但都在同一个 Unit / 同一 | ||
| 356 | +次提交内完成,不对外暴露中间态。 | ||
| 357 | + | ||
| 358 | +**完成定义(Definition of Done)**: | ||
| 359 | + | ||
| 360 | +- [ ] `python3 -m unittest src.test_skill_query -v` 全绿,含重写后的 `GetSkillContentTest`。 | ||
| 361 | +- [ ] `python3 -m unittest src.test_backend_server.SkillRouteTest -v` 全绿,含重写后的 | ||
| 362 | + `test_content_*` 与新增的越界路由层测试。 | ||
| 363 | +- [ ] `python3 -m unittest discover -s src` 整体全绿(确认没有破坏其他模块,特别是 | ||
| 364 | + `agent_profile.py`/`skill_update.py` 相关测试)。 | ||
| 365 | +- [ ] 手工验证(本仓库前端无自动化测试基建,与 PRD §8 一致):浏览器打开 | ||
| 366 | + `http://127.0.0.1:8420/#/skills` → 点击任一已迁移 Skill → `#/skills/:id/audit` 页面同时 | ||
| 367 | + 显示“Skill 内容”真实文本面板与“迭代记录”面板,二者互不覆盖。 | ||
| 368 | + | ||
| 369 | +**验收标准(对齐 PRD §9 AC1-8,逐条回链)**: | ||
| 370 | + | ||
| 371 | +| PRD AC | 验证方式 | | ||
| 372 | +|---|---| | ||
| 373 | +| AC1 真实内容以 `<pre>` 展示 | 手工验证 + `GetSkillContentTest.test_returns_real_file_content` | | ||
| 374 | +| AC2 变更审计面板行为不变 | `SkillRouteTest.test_audit_log_*` 保持通过(未改动) | | ||
| 375 | +| AC3 两面板并列、互不依赖 | 手工验证;`mountSkillAuditLog` 两个 fetch 保持独立发起 | | ||
| 376 | +| AC4 文件缺失显示“内容文件不可用”,不 500 | `GetSkillContentTest` 缺失文件用例(`content is None`,无异常)+ `SkillRouteTest` 新增的路由层 200/null 用例 | | ||
| 377 | +| AC5 skill 不存在 → 整页错误态,两面板都不渲染,404 | `SkillRouteTest.test_content_nonexistent_skill_returns_404` 重写版 + 前端 `showNotFound()` 手工验证 | | ||
| 378 | +| AC6 越界 `file_path` 不读出目录外内容,400 非 500 | `GetSkillContentTest` 路径穿越两个用例(改断言为 `SkillPathTraversalError`)+ `SkillRouteTest` 新增的 400 路由层用例 | | ||
| 379 | +| AC7 既有测试全通过、断言不被无理由篡改 | 见 §0 裁决:仅重写与本次契约变更直接相关的用例,其余原样保留 | | ||
| 380 | +| AC8 前端无新第三方依赖 | Code Review 检查 diff,不新增 `<script src>` | | ||
| 381 | + | ||
| 382 | +--- | ||
| 383 | + | ||
| 384 | +## 7. 追溯矩阵(原始要求 → 设计 → 证据) | ||
| 385 | + | ||
| 386 | +| PRD 条款 | 本 SDD 落点 | 证据 | | ||
| 387 | +|---|---|---| | ||
| 388 | +| BRD 要求1(新增读取真实内容函数) | §2.3 `get_skill_content()` 重写 | `GetSkillContentTest` | | ||
| 389 | +| BRD 要求2(并列展示,不替换迭代面板) | §4 前端设计 | 手工验证 + `renderSkillAuditLog` DOM 不变 | | ||
| 390 | +| BRD 要求3(纯 `<pre>`,沿用 `v_artifact_browser` 约定) | §4 | Code Review:无新依赖 | | ||
| 391 | +| BRD 要求4(防路径穿越) | §3 安全设计 + §2.3 `SkillPathTraversalError` | `GetSkillContentTest` 路径穿越用例 + `SkillRouteTest` 400 用例 | | ||
| 392 | +| PRD §4.1 决策一(裸 `:skill_id` 路由) | §2.1/§2.2 | `SkillRouteTest` 新 URL 断言 | | ||
| 393 | +| PRD §4.2 决策二(支持文件仅列清单) | §2.3 `supporting_files` 枚举 + §4 前端渲染 | `GetSkillContentTest` 支持文件枚举用例 | | ||
| 394 | +| PRD §6 安全需求表 | §3 | 同上路径穿越用例 | | ||
| 395 | +| PRD §7 兼容性 | §0 裁决 + §5 | 其余四个既有接口测试原样保留通过 | | ||
| 396 | + | ||
| 397 | +--- | ||
| 398 | + | ||
| 399 | +## 8. 风险与假设 | ||
| 400 | + | ||
| 401 | +- **假设**:当前仓库内所有已迁移 Skill 目录下确实只有 `SKILL.md` 一个文件(PRD §4.2 前提)—— | ||
| 402 | + 若后续某个 Skill 目录下真的出现了 `references/`/`scripts/` 子文件,`supporting_files` 枚举逻 | ||
| 403 | + 辑已经能正确列出,不需要额外改动,属于已覆盖的场景,非阻塞性未知。 | ||
| 404 | +- **风险(低)**:删除 `SkillContentMissingError` 类是一次破坏性重命名,需在改动前用 | ||
| 405 | + `grep -rn SkillContentMissingError` 确认调用面仅限本次改动涉及的两个文件,避免遗漏。 | ||
| 406 | +- **风险(低)**:`docs/wireframes/v_skill_audit_log.html` 若被其他未来 Unit 引用为字段名参照 | ||
| 407 | + 源,未同步更新会造成新的文档-代码漂移——已在 §6 顺手项中标注,建议一并处理但不作为阻断本 | ||
| 408 | + Unit 完成的硬性条件。 |