# SDD：Skill 详情页展示真实内容（`v_skill_audit_log` 契约收口） 状态：Approved PRD 已定稿，本 SDD 据其落地设计。 对应 PRD：《PRD：Skill 详情页展示真实内容》（本任务 goal 的同批交付物）。 涉及模块：`src/skill_query.py`、`src/skill_update.py`（只读引用，不修改其写路径逻辑）、`src/backend_server.py`、`frontend/app.js`、对应测试文件。 --- ## 0. 前置说明：PRD 根因表述与当前代码基线的差异（必读，决定本 Unit 的真实范围） PRD 背景部分描述的根因是：“`src/skill_query.py` 目前只有 `list_skills()` 和 `get_skill_audit_log()`，没有任何函数读取 `skills//SKILL.md` 的真实内容”。 **经代码核实（2026-08-13），这个前提已不成立**：当前 `src/skill_query.py` 已经存在 `get_skill_content()`（含 `SkillContentMissingError`、两层路径包含性校验、`_skill_file_path` 复用），`src/backend_server.py` 已经存在 `GET /api/skills/:skill_id/content` 路由和 `_handle_skill_content`，`frontend/app.js` 的 `renderSkillAuditLog`/`mountSkillAuditLog` 也已经 在渲染一个独立的“Skill 内容”面板，`src/test_skill_query.py`（`GetSkillContentTest` 一类用例）和 `src/test_backend_server.py::SkillRouteTest`（`test_content_*` 系列）也都已经覆盖了这套实现。 换句话说：**“页面点击 Skill 不显示真实内容”这个用户可见的 bug，字面意义上可能已经不存在**—— 打开 `#/skills/:skill_id/audit` 今天就能看到“Skill 内容”面板和真实文本。但本 PRD 在“背景”之外 还在 §4.1/§4.2/FR-1/FR-2/FR-3 里做出了一组具体的**设计决策**，而这些决策与当前实现在契约形状上 **不一致**： | 维度 | 当前代码实际行为 | PRD 决策要求的行为 | |---|---|---| | 路由形态 | `GET /api/skills/:skill_id/content`（`/content` 后缀） | `GET /api/skills/:skill_id`（与 `/audit` 并列的裸资源路由，§4.1） | | 支持文件字段名 | `support_files` | `supporting_files`（§4.1 JSON 示例、FR-1） | | 响应体字段完整性 | 仅 `{skill_id, content, support_files}` | 还需包含 `owner_org_node`/`file_path`/`current_git_commit_sha`/`source`/`migrated_from_profile`/`status`（§4.1） | | 磁盘文件缺失时的语义 | 抛 `SkillContentMissingError` → 404 `CONTENT_MISSING` | 200，`content: null`，不抛异常（FR-1、§6） | | 路径穿越（`file_path` 越界）时的语义 | 同样抛 `SkillContentMissingError` → 404 `CONTENT_MISSING`（与“文件缺失”共用同一个异常/错误码，前端无法区分） | 专门的 `SkillPathTraversalError` → 400 `INVALID_SKILL_ID`（§4.1、FR-2、§6 安全表） | | skill 不存在 vs 内容缺失的页面态 | “Skill 内容”面板独立显示提示文案，“变更审计”面板独立显示 404 横幅；skill 完全不存在时两个面板各自处理，互不联动 | skill 完全不存在时两个面板共用同一条顶部错误横幅、都不渲染（FR-3、AC5） | **本 Unit 的真实工作内容因此是“契约收口”，不是“从零实现”**：复用已经写好的读取/展示骨架， 把上表右列的决策真正落地，同时保留骨架里仍然正确、PRD 未要求改变的部分（`_skill_file_path` 复用、`realpath`+前缀包含性校验的整体思路、`skill_iteration` 面板完全不动）。 PRD §7 写道“现有 `src/test_skill_query.py`、`src/test_backend_server.py::SkillRouteTest` 中的既 有用例必须全部保持通过，不允许为了实现新功能而修改其断言”——这句话是在“该函数尚不存在”这个 （已不成立的）前提下写的，字面执行会与上表要求的契约变更直接冲突（例如 `support_files` 必须 改名为 `supporting_files`，测试断言不可能不变）。本 SDD 对此的裁决（决策，非独走）： > **§7 的“既有用例不可修改”原则，适用范围收窄为“不测试 `get_skill_content`/`/content` > 路由本身契约形状”的用例**（即 `test_lists_all_skills`、`test_skill_with_no_iterations_returns > _empty_list`、`test_full_update_diff_rollback_cycle`、`test_rollback_*`、`test_diff_*`、 > `test_audit_log_*` 等——这些测试 `list_skills`/`get_skill_audit_log`/`update_skill`/ > `rollback_skill`/`get_diff`，与本次契约变更无关，必须保持通过不动）。 > 而 `GetSkillContentTest` 全部用例、`SkillRouteTest.test_content_*` 三个用例，测试的正是本次要 > 改变契约形状的那个函数/路由本身，**必须随契约一起更新**——这不是“为了让新功能过测试而放宽 > 断言”，是给同一段代码换了一份新的、PRD 已批准的验收契约。Code Agent 落地时应把这几个测试方 > 法**重写为断言新契约**，而不是删除或跳过。 --- ## 1. 架构映射（M/V/C，遵循仓库既有 MVCP 记法） - **M（数据）**：不变。`skill` 表 schema 不动；磁盘上 `skills//SKILL.md` 及可能的 支持文件是“数据”的另一半（platform asset，`skill_query.py` 顶部注释已有此定位），本次只读。 - **C（控制器/函数）**： - `src/skill_query.py::get_skill_content()` —— 改造对象，函数名不变，签名`(conn, skill_id, repo_path=None)` 不变，返回值形状和异常类型变。 - 新增 `src/skill_query.py::SkillPathTraversalError`。 - `src/backend_server.py::_handle_skill_content` —— 改造对象；路由注册从 `/content` 后缀改为 裸 `skill_id` 分支，函数名可保留（或按第 3.2 节改名 `_handle_skill_get`，由实现选择，只要 路由行为符合 §3.2 的匹配顺序）。 - `src/skill_update.py::_skill_file_path` —— 只读复用，不修改。 - **V（视图）**：`frontend/app.js::renderSkillAuditLog`/`mountSkillAuditLog` —— 改造对象，DOM 结构基本保留（“Skill 内容”面板已经存在，位置、标题不变），改的是 fetch 的 URL、字段名、以及 错误态/`content===null` 态的呈现逻辑。对应 wireframe：`docs/wireframes/v_skill_audit_log.html` （如该 wireframe 里的字段名/结构与新契约不一致，一并同步，属于本 Unit 的收尾项，不需要单独 开 Unit）。 --- ## 2. 目标契约设计 ### 2.1 `GET /api/skills/:skill_id` 响应契约（替代旧 `GET /api/skills/:skill_id/content`） 请求：`GET /api/skills/skill-1` 成功（200）： ```json { "skill_id": "skill-1", "owner_org_node": "coder", "file_path": "skills/skill-1", "current_git_commit_sha": "unknown", "source": "migrated_from_hermes", "migrated_from_profile": "code", "status": "active", "content": "...SKILL.md 全文，UTF-8；文件缺失时为 null...", "supporting_files": ["references/foo.md", "scripts/bar.py"] } ``` 错误： | 情形 | HTTP | body | |---|---|---| | `skill_id` 在 `skill` 表中不存在 | 404 | `{"error": "NOT_FOUND"}` | | `skill.file_path` 解析出的绝对路径落在 `repo_root/skills/` 之外（无论是历史数据脏值还是未来 被篡改） | 400 | `{"error": "INVALID_SKILL_ID"}` | | 上述两种情形都不成立，但磁盘 `SKILL.md` 不存在（DB 有行，文件缺失/尚未迁移） | **200**，`content: null` | 不是错误 | 旧路由 `GET /api/skills/:skill_id/content` 直接移除（不保留兼容别名——PRD 明确是“新增一个兄弟 接口”而非“新增一个后缀”，且当前唯一调用方就是本仓库自己的前端，随本 Unit 一并切换，没有第三方 消费者需要兼容期）。 ### 2.2 路由匹配顺序（`src/backend_server.py::do_GET`） 在既有 `_SKILLS_PREFIX` 相关分支中，新分支必须放在 `/audit` 分支**之后**、旧 `/content` 分支 **原地替换**： ```python if parsed.path == "/api/skills": self._handle_skill_list(); return if parsed.path == "/api/skills/diff": self._handle_skill_diff(parsed.query); return if parsed.path.startswith(_SKILLS_PREFIX) and parsed.path.endswith("/audit"): ...; self._handle_skill_audit_log(skill_id); return # 原来这里是 endswith("/content") 分支 -- 整段删除，替换为下面的裸 skill_id 分支 if parsed.path.startswith(_SKILLS_PREFIX): skill_id = parsed.path[len(_SKILLS_PREFIX):] if skill_id and "/" not in skill_id: self._handle_skill_get(skill_id) return ``` 这与 `_ARTIFACTS_PREFIX`（`GET /api/artifacts/:artifact_id`）已有的“剩余路径不含 `/` 才命中”写 法完全同构，不发明新的路由匹配范式。`POST .../rollback` 分支在 `do_POST` 里，`_SKILLS_PREFIX` 复用不冲突，无需改动。 ### 2.3 `get_skill_content()` 新契约 ```python class SkillPathTraversalError(Exception): """INVALID_SKILL_ID -- skill.file_path 解析出的绝对路径落在 repo_root/skills/ 之外。永远在打开任何文件之前抛出（fail closed）。与 SkillNotFoundError （skill_id 在 DB 里就查不到）和"磁盘文件缺失但路径合法"（不再是异常，见 下方 content=None）是三种互斥的独立情形。""" def get_skill_content(conn, skill_id, repo_path=None): row = conn.execute( "SELECT owner_org_node, file_path, current_git_commit_sha, source, " "migrated_from_profile, status FROM skill WHERE skill_id = ?", (skill_id,) ).fetchone() if row is None: raise SkillNotFoundError(skill_id) owner_org_node, file_path, sha, source, migrated_from_profile, status = row repo_root = repo_path or _REPO_ROOT skills_root = os.path.realpath(os.path.join(repo_root, "skills")) rel_path = skill_update._skill_file_path(file_path) abs_path = os.path.realpath(os.path.join(repo_root, rel_path)) if os.path.commonpath([abs_path, skills_root]) != skills_root: raise SkillPathTraversalError(skill_id) # 400, 未打开任何文件 try: with open(abs_path, "r", encoding="utf-8", errors="replace") as f: content = f.read() except OSError: content = None # 200, 不是异常 -- 与 v_artifact_browser 的 content:null 约定对齐 skill_dir = os.path.dirname(abs_path) supporting_files = [] for dirpath, _dirnames, filenames in os.walk(skill_dir): # 目录不存在时 os.walk 直接返回空，不报错 for name in filenames: full_path = os.path.realpath(os.path.join(dirpath, name)) if full_path == abs_path: continue supporting_files.append(os.path.relpath(full_path, skill_dir)) supporting_files.sort() return { "skill_id": skill_id, "owner_org_node": owner_org_node, "file_path": file_path, "current_git_commit_sha": sha, "source": source, "migrated_from_profile": migrated_from_profile, "status": status, "content": content, "supporting_files": supporting_files, } ``` 关键变化点（相对当前代码）： 1. `SELECT` 从只取 `file_path` 改为取完整 skill 行（新增 5 个列），字段直接拼进返回 dict——不 走 `list_skills()` 二次查询，避免一次请求两次打库；这是“由实现选择”范围内的合理做法（PRD FR-2 允许在 `skill_query` 层或 backend handler 层拼装，此处选择前者，因为已经有 skill 行在 手，没有理由再查一次）。 2. 路径包含性校验失败（layer 1）：异常类型从 `SkillContentMissingError` 改为 `SkillPathTraversalError`，语义从“内容不可用”收紧为“非法输入拒绝”。 3. 文件打开失败（layer 2，`OSError`）：不再 `raise`，改为 `content = None` 后继续往下执行（还 要继续枚举 `supporting_files`，即使 `SKILL.md` 本身缺失，目录里仍可能有其他文件——虽然当前 仓库场景下不会出现“文件缺失但目录有其他文件”的组合，但函数逻辑上不应该因为主文件缺失就跳过 目录枚举）。 4. `SkillContentMissingError` 类整体删除（不再有任何调用点会抛出它）——删除时先用 `grep -rn SkillContentMissingError src/` 确认调用方只有 `backend_server.py` 一处并同步改掉， 避免留下引用不存在符号的 `except` 分支。 ### 2.4 `_handle_skill_content` → 改为裸 skill_id 的 handler ```python def _handle_skill_get(self, skill_id): """GET /api/skills/:skill_id -- Skill 详情：DB 行字段 + 真实 SKILL.md 内容 + 支持文件清单，与 /audit（迭代审计）并列，两者响应体互不影响。""" try: with self.server.conn_lock: result = skill_query.get_skill_content(self.server.conn, skill_id, repo_path=self.server.skill_repo_path) except skill_query.SkillNotFoundError: self._send_json(404, {"error": "NOT_FOUND"}) except skill_query.SkillPathTraversalError: self._send_json(400, {"error": "INVALID_SKILL_ID"}) else: self._send_json(200, result) ``` --- ## 3. 安全设计：路径穿越两层防御（沿用既有思路，收紧输出语义） 防御结构本身（不变，继续沿用当前代码已经做对的部分）： 1. **输入面**：`skill_id` 永远只用于参数化 SQL `WHERE skill_id = ?`，从不直接拼接文件系统路 径——这一点当前代码已经做到，本次不引入回退。 2. **路径来源**：文件路径来自可信的 `skill.file_path` DB 列 + `skill_update._skill_file_path()` 的既有解析规则（复用，不重写），不是来自 `skill_id` 本身。 3. **纵深校验**：`os.path.realpath` 双方都规范化后做 `os.path.commonpath` 前缀包含性判断——沿 用当前代码已验证过（含 `skills-evil/` 前缀误判的测试用例）的写法，不换算法。 **唯一变化**：越界情形下的对外语义从“和文件缺失共用同一个 404”变成独立的 400 与独立异常类 型。测试层面新增/沿用（当前 `GetSkillContentTest` 里的 `test_traversal_via_directory_component_returns_content_missing`、 `test_traversal_via_relative_dotdot_in_file_path` 这两个已存在的用例本身逻辑正确，只需把 `assertRaises(SkillContentMissingError)` 改成 `assertRaises(SkillPathTraversalError)`，测试意图和覆盖面完全不用重新设计）。PRD §6 要求的“插 入 `file_path` 被篡改为 `"../../etc/"` 的记录，断言拒绝读取而不是把上层目录内容读出来返回”这 条，当前测试已经用哨兵式的“断言抛异常”方式间接满足（异常发生在 `open()` 之前，物理上不可能读 出内容）；如需更直白的证据，可以在越界目录里放一个内容各异的哨兵文件，断言返回值里完全不含该 文件内容/文件名——非强制，现有的“抛异常且从不 open”已经是充分证据。 --- ## 4. 前端设计（`frontend/app.js`） `renderSkillAuditLog` 的 DOM 结构**不需要改**（“Skill 内容”面板、`#skill_content`、 `#skill_audit_error`、`#skill_audit_list` 都已经是 PRD 要求的样子）。改动全部在 `mountSkillAuditLog` 内部： ```js function mountSkillAuditLog(skillId) { const listEl = document.getElementById("skill_audit_list"); const errorEl = document.getElementById("skill_audit_error"); const contentEl = document.getElementById("skill_content"); function showNotFound() { // FR-3: skill 完全不存在时，两个面板共用同一条顶部错误横幅，都不渲染自己的内容 contentEl.innerHTML = ""; listEl.innerHTML = ""; errorEl.textContent = "该 Skill 不存在"; errorEl.style.display = "block"; } fetch("/api/skills/" + encodeURIComponent(skillId)).then(async (resp) => { if (resp.status === 404) { showNotFound(); return; } if (resp.status !== 200) { // 400 INVALID_SKILL_ID 等：现实中不可达（skill_id 已先过 DB 校验）， // 但必须优雅降级，不能让面板空白报错 contentEl.innerHTML = '
Skill 内容暂不可用
'; return; } const skill = await resp.json(); const supportHtml = skill.supporting_files.length ? '
支持文件：' + skill.supporting_files.map(escapeHtml).join("、") + "
" : '
无支持文件
'; contentEl.innerHTML = supportHtml + '
' + (skill.content == null ? "（内容文件不可用）" : escapeHtml(skill.content)) + "
"; }); async function refresh() { const resp = await fetch("/api/skills/" + encodeURIComponent(skillId) + "/audit"); if (resp.status === 404) { showNotFound(); return; } const iterations = await resp.json(); if (iterations.length === 0) { listEl.innerHTML = '
该 Skill 尚无自主迭代记录
'; return; } // ...其余迭代记录渲染逻辑完全不变，原样保留... } refresh(); } ``` 要点： - 内容渲染 `skill.content == null ? "（内容文件不可用）" : escapeHtml(skill.content)` 与 `showPreview()` 里 `artifact.content == null ? "（文件不可用）" : escapeHtml(artifact.content)` 同构（措辞留用当前 Skill 面板已经在用的“（内容文件不可用）”，不必和 artifact 面板逐字一致， PRD 原文也是这个措辞）——继续不引入任何 Markdown/代码高亮库。 - `supporting_files` 字段名同步改名（原 `support_files`）。 - 两个 fetch（`/api/skills/:id` 和 `/api/skills/:id/audit`）保持并行发起、独立渲染，任一方失败 不阻塞另一方——这一点当前代码已经是这样，不用改并行结构，只需让“skill 完全不存在”这一种情形 从两个面板分别处理收敛成共用 `showNotFound()`。 - `docs/wireframes/v_skill_audit_log.html` 若存在与新字段名/新面板结构不一致之处，随手同步（低 成本收尾项，不需要单独验收）。 --- ## 5. 兼容性与回归范围 - `GET /api/skills`、`GET /api/skills/:skill_id/audit`、`GET /api/skills/diff`、 `POST /api/skills/:skill_id/rollback` 四个接口的路由匹配、响应契约、行为完全不变。 - `GET /api/skills/:skill_id/content` 直接移除（无兼容层，见 §2.1 理由）。 - `src/test_skill_query.py` 中 `ListSkillsTest`、`GetSkillAuditLogTest`、`GetDiffTest` 三个测试 类原样保留；`GetSkillContentTest` 类里的用例按 §0 裁决重写为新契约的断言，覆盖面不变（正常 内容、缺失文件、Skill 不存在、路径穿越两种触发方式、支持文件枚举）。 - `src/test_backend_server.py::SkillRouteTest` 中 `test_list_returns_seeded_skill`、 `test_audit_log_*`、`test_diff_*`、`test_full_update_diff_rollback_cycle`、`test_rollback_*` 原样保留；`test_content_*` 三个方法重写为新契约（URL 从 `/content` 改为裸 id，字段名改，缺失 文件场景断言从 404 改为 200+`content:null`）。 --- ## 6. Development Unit 分解 单一纵向 Unit，理由：三层改动（`skill_query.py` 契约 → `backend_server.py` 路由/handler → `app.js` 消费方）互为强依赖，响应体形状是同一个契约的三处镜像，拆成多个 Unit 只会制造“后端改 完前端还在用旧字段名”的中间不可验收态，不产生任何独立可交付价值。规模上（3 个源文件改动 + 2 个测试文件改动，均为已有文件的局部修改，无新文件）单个 Code Agent 一次连贯实现完全可以覆盖， 符合“合格 Unit 能交给单个 Code Agent，在一次连贯实现与评审周期中完成”的标准。 ### DU-01：Skill 详情内容契约收口（bug fix） **类型**：Bug 修复（纵向，覆盖 M 不变 / C 两处 / V 一处）。 **范围**： 1. `src/skill_query.py`：`get_skill_content()` 按 §2.3 重写；新增 `SkillPathTraversalError`； 删除 `SkillContentMissingError`（先确认无其他调用方）。 2. `src/backend_server.py`：按 §2.2 调整 `do_GET` 路由匹配顺序（移除 `/content` 后缀分支，新增 裸 `skill_id` 分支）；`_handle_skill_content` 按 §2.4 重写为 `_handle_skill_get`（或保留原函 数名，仅改内部逻辑与调用点——由实现选择，行为以 §2.1/§2.4 为准）。 3. `frontend/app.js`：`mountSkillAuditLog` 按 §4 重写；字段名 `support_files`→ `supporting_files`；skill 不存在时两个面板共用顶部横幅。 4. 测试：`src/test_skill_query.py::GetSkillContentTest`、 `src/test_backend_server.py::SkillRouteTest.test_content_*` 按 §0/§5 重写为新契约断言；新增 “磁盘文件缺失 → 200 + content:null”“路径穿越 → 400 + INVALID_SKILL_ID”两个显式路由层测试 （当前 `SkillRouteTest` 里没有覆盖越界场景的路由层用例，仅 `skill_query` 单元层有——PRD §8 要求两层都要有覆盖，这是本 Unit 需要真正新增的测试，不是重写）。 5. 顺手项（非必须但成本极低，同一次改动一起做）：核实并按需更新 `docs/wireframes/v_skill_audit_log.html` 中与新字段名不一致的位置。 **不在范围内**（继承 PRD 非目标）：`skill_update.py`/迁移逻辑本身；`skill_iteration` 查询逻 辑；支持文件的内容预览；Markdown 渲染引入。 **依赖**：无对其他未完成 Unit 的依赖；三处改动内部顺序建议 `skill_query.py` → `backend_server.py` → `app.js`（每一步都可独立跑对应层的测试验证），但都在同一个 Unit / 同一 次提交内完成，不对外暴露中间态。 **完成定义（Definition of Done）**： - [ ] `python3 -m unittest src.test_skill_query -v` 全绿，含重写后的 `GetSkillContentTest`。 - [ ] `python3 -m unittest src.test_backend_server.SkillRouteTest -v` 全绿，含重写后的 `test_content_*` 与新增的越界路由层测试。 - [ ] `python3 -m unittest discover -s src` 整体全绿（确认没有破坏其他模块，特别是 `agent_profile.py`/`skill_update.py` 相关测试）。 - [ ] 手工验证（本仓库前端无自动化测试基建，与 PRD §8 一致）：浏览器打开 `http://127.0.0.1:8420/#/skills` → 点击任一已迁移 Skill → `#/skills/:id/audit` 页面同时 显示“Skill 内容”真实文本面板与“迭代记录”面板，二者互不覆盖。 **验收标准（对齐 PRD §9 AC1-8，逐条回链）**： | PRD AC | 验证方式 | |---|---| | AC1 真实内容以 `
` 展示 | 手工验证 + `GetSkillContentTest.test_returns_real_file_content` |
| AC2 变更审计面板行为不变 | `SkillRouteTest.test_audit_log_*` 保持通过（未改动） |
| AC3 两面板并列、互不依赖 | 手工验证；`mountSkillAuditLog` 两个 fetch 保持独立发起 |
| AC4 文件缺失显示“内容文件不可用”，不 500 | `GetSkillContentTest` 缺失文件用例（`content is None`，无异常）+ `SkillRouteTest` 新增的路由层 200/null 用例 |
| AC5 skill 不存在 → 整页错误态，两面板都不渲染，404 | `SkillRouteTest.test_content_nonexistent_skill_returns_404` 重写版 + 前端 `showNotFound()` 手工验证 |
| AC6 越界 `file_path` 不读出目录外内容，400 非 500 | `GetSkillContentTest` 路径穿越两个用例（改断言为 `SkillPathTraversalError`）+ `SkillRouteTest` 新增的 400 路由层用例 |
| AC7 既有测试全通过、断言不被无理由篡改 | 见 §0 裁决：仅重写与本次契约变更直接相关的用例，其余原样保留 |
| AC8 前端无新第三方依赖 | Code Review 检查 diff，不新增 `