宜博 / lemongraph

Commit c788bbacbef7234a2925e7080813a6de5a4ec154

Authored by graphlemon
1 parent 2c19914b

graphlemon: prd artifact for task b243dfe8-4da8-49bd-895b-8b9e9ed17280

Inline Side-by-side
Showing 1 changed file with 209 additions and 0 deletions
.graphlemon/runs/b243dfe8-4da8-49bd-895b-8b9e9ed17280/prd.md 0 → 100644
  View file @c788bba
  1 +# PRD：Skill 详情页展示真实内容
  2 +
  3 +## 1. 背景与问题
  4 +
  5 +`http://127.0.0.1:8420/#/skills/:skill_id/audit`（`v_skill_audit_log`）目前只查询并展示
  6 +`skill_iteration` 表（“变更审计”面板）。当某个 Skill 还没有任何自主迭代记录时（当前所有从
  7 +Hermes profile 迁移来的 Skill 都是这种状态），页面只显示“该 Skill 尚无自主迭代记录”，用户看
  8 +不到 Skill 本身写了什么——即 `skills/<skill_id>/SKILL.md` 的真实文本内容。
  9 +
  10 +根因：`src/skill_query.py` 只有 `list_skills()`（读 `skill` 表）和 `get_skill_audit_log()`（读
  11 +`skill_iteration` 表），没有任何函数读取磁盘上 `skills/<skill_id>/SKILL.md` 的真实内容。这批真
  12 +实文件是最近一次迁移任务才创建的（`skill_update.py` 旧注释“`skills/<skill_id>/` doesn't exist
  13 +as real files”已过期，需在实现时一并核实/更新该注释，但不属于本次改动范围本身）。
  14 +
  15 +本次修复是纯粹的“读取 + 展示”接入，不改变 Skill 迁移逻辑，不改变现有迭代审计逻辑。
  16 +
  17 +## 2. 目标（Goals）
  18 +
  19 +- G1：`v_skill_audit_log` 页面新增“Skill 内容”面板，与既有“变更审计”面板并列展示，用户打开任
  20 + 意 Skill 详情页都能看到该 Skill 的真实 `SKILL.md` 文本内容。
  21 +- G2：后端新增只读接口，暴露 Skill 内容 + 支持文件清单，且对 `skill_id` 这个用户可控输入做路
  22 + 径穿越防护。
  23 +- G3：内容渲染方式与 `v_artifact_browser` 保持一致（纯 `<pre>` 文本，不引入 Markdown/代码高亮
  24 + 库），维持“纯 vanilla JS、无构建步骤”的既有约定。
  25 +
  26 +## 3. 非目标（Non-goals）
  27 +
  28 +- 不改动 `skill_update.py` / Skill 迁移脚本的任何逻辑。
  29 +- 不改动 `get_skill_audit_log()` 现有查询逻辑、现有 `/api/skills/:skill_id/audit` 响应契约、
  30 + 现有“迭代记录”面板的 DOM 结构——两个信息源并列，不做二选一或合并覆盖。
  31 +- 不提供 Skill 内容的在线编辑/写入能力（本次是只读展示，写入仍只能走
  32 + `c_skill_update`/`c_skill_rollback` 现有链路）。
  33 +- 不引入 Markdown 渲染、代码高亮或任何新的前端第三方依赖。
  34 +- 不做支持文件（`references/`/`templates/`/`scripts/` 等）的内容预览——本次只做文件清单展
  35 + 示，点击查看子文件内容不在范围内（当前仓库内所有已迁移 Skill 目录下都还没有这类子文件，属
  36 + 于面向未来的占位展示，先满足“看得见有哪些文件”，内容预览留待真实需求出现时再建，避免过度设
  37 + 计）。
  38 +- 不做通用二进制文件预览。
  39 +
  40 +## 4. 关键设计决策（解决 BRD 遗留的两个开放问题）
  41 +
  42 +### 4.1 决策一：新增接口的形态
  43 +
  44 +新增 `GET /api/skills/:skill_id`（Skill 详情接口），与已有的 `GET /api/skills`（列表）、
  45 +`GET /api/skills/:skill_id/audit`（迭代审计）并列，不复用/不修改 `/audit` 的响应契约。
  46 +
  47 +理由：
  48 +- 参照本仓库已有先例 `GET /api/artifacts/:artifact_id`——列表接口返回摘要，详情接口返回完整
  49 + 内容（含 `content` 字段，`null` 表示文件不可用）。`/skills/:skill_id` 遵循同一 REST 资源语
  50 + 义：Skill 本身的详情 vs. Skill 的迭代审计子资源，职责天然分离。
  51 +- 修改 `/audit` 的响应体形状会牵动其现有测试（`SkillRouteTest`）和前端既有渲染逻辑，增加不必
  52 + 要的回归面；新增一个兄弟接口是零风险的纵向增量。
  53 +- 前端页面通过 `Promise.all` 并行请求 `/api/skills/:skill_id` 与
  54 + `/api/skills/:skill_id/audit`（参照 `mountArtifactsOverviewPanel` 已有的并行请求写法），两
  55 + 个面板各自独立渲染、独立失败态，符合“两个 independent 信息源”的要求。
  56 +
  57 +`GET /api/skills/:skill_id` 响应体：
  58 +
  59 +```json
  60 +{
  61 + "skill_id": "code-agent-engineering",
  62 + "owner_org_node": "coder",
  63 + "file_path": "skills/code-agent-engineering/",
  64 + "current_git_commit_sha": "unknown",
  65 + "source": "migrated_from_hermes",
  66 + "migrated_from_profile": "code",
  67 + "status": "active",
  68 + "content": "<SKILL.md 全文文本，UTF-8；文件不存在时为 null>",
  69 + "supporting_files": ["references/foo.md", "scripts/bar.py"]
  70 +}
  71 +```
  72 +
  73 +- 404 `{"error": "NOT_FOUND"}`：`skill_id` 在 `skill` 表中不存在（与 `/audit` 现有 404 行为保
  74 + 持一致的错误形状）。
  75 +- `content: null`：`skill` 表中有该行，但对应的 `SKILL.md` 文件在磁盘上缺失（与
  76 + `v_artifact_browser` 中 `artifact.content == null` 的既有约定一致，不视为错误，200 返回）。
  77 +
  78 +### 4.2 决策二：支持文件的呈现方式
  79 +
  80 +`supporting_files` 是一个字符串数组，元素为相对 Skill 目录的相对路径（如
  81 +`"references/foo.md"`），按字典序排序；只列出文件本身是否存在，不读取、不返回其内容。前端只
  82 +渲染成一个纯文本清单（`<ul>`/`<pre>`，非可点击的预览入口）；目录为空时显示“无支持文件”提示
  83 +文案，不请求也不渲染额外内容面板。
  84 +
  85 +理由：当前仓库所有已迁移 Skill 目录下都只有 `SKILL.md` 一个文件，`references/`/`templates/`/
  86 +`scripts/` 完全是面向未来的假设场景；先给出“存在哪些文件”的最小可见性即可满足 BRD 要求
  87 +（“判断要不要一并列出/展示”），把内容预览这个更大的功能面留给真实出现该场景时再单独立项，避
  88 +免为不存在的需求建可点击的子路由/子预览逻辑。
  89 +
  90 +## 5. 功能需求（Functional Requirements）
  91 +
  92 +### FR-1：`src/skill_query.py` 新增 `get_skill_content(conn, skill_id, repo_path=None)`
  93 +
  94 +- 复用现有 `SkillNotFoundError`：先按 `skill_id` 查 `skill` 表（参数化 SQL，与
  95 + `get_skill_audit_log()` 完全相同的校验前置模式），查不到直接抛出，不触碰文件系统。
  96 +- **不得**用 `skill_id` 直接拼接文件系统路径（例如
  97 + `os.path.join(repo_root, "skills", skill_id, "SKILL.md")` 这种写法是本次要杜绝的漏洞模式）。
  98 + 必须复用查到的 `skill.file_path` 列值来解析真实路径——这是 `skill_update.py` 的
  99 + `_skill_file_path()` 已经在用的解析规则（`.md` 结尾直接用，否则拼接 `SKILL.md`）。为避免两
  100 + 处实现漂移，`skill_query.py` 应直接复用 `skill_update._skill_file_path`（导入复用），不要重
  101 + 写一份等价逻辑。
  102 +- 纵深防御：即便路径已经来自 DB 而非直接来自 `skill_id`，仍需在打开文件前做一次路径包含性校
  103 + 验——对解析出的绝对路径和 `repo_root/skills` 的绝对路径都做 `os.path.realpath`，确认前者位
  104 + 于后者之内（`os.path.commonpath` 或等价前缀比较，注意分隔符边界，避免
  105 + `skills-evil/` 这类前缀误判）。校验失败时抛出新增的 `SkillPathTraversalError`，**不读取任何
  106 + 文件**，即“失败关闭”（fail closed）。这一层是防止 `skill.file_path` 列未来被写入非预期值
  107 + （人为改库、迁移脚本 bug 等）时仍然造成越界读取。
  108 +- 读取 `SKILL.md`：文件存在则以 UTF-8 读取全文返回；文件不存在（DB 有行但磁盘文件缺失，例如
  109 + 尚未迁移完成）时 `content` 置为 `None`，不抛异常（对齐 `v_artifact_browser` 的“文件不可用”
  110 + 既有约定）。
  111 +- 枚举支持文件：遍历 Skill 目录（`os.walk`，跳过 `SKILL.md` 自身），返回排序后的相对路径列
  112 + 表；目录本身不存在时返回空列表，不报错。
  113 +- 返回值：`dict`，字段见 4.1 中的 JSON 结构（不含 `skill_id` 以外的 DB 字段拼装可以留给
  114 + backend handler 做，也可以在本函数里一并拼好——由实现选择，只要最终 API 响应体符合 4.1 的
  115 + 字段集合）。
  116 +
  117 +### FR-2：`src/backend_server.py` 新增路由 `GET /api/skills/:skill_id`
  118 +
  119 +- 路由匹配顺序：先匹配 `.../audit` 和 `.../rollback` 后缀（现有逻辑不变），都不命中且路径以
  120 + `_SKILLS_PREFIX` 开头、且不等于 `/api/skills`（列表）、不等于 `/api/skills/diff` 时，落到新
  121 + 的详情 handler，把剩余路径段当作 `skill_id`。
  122 +- Handler 调用 `skill_query.get_skill_content`：
  123 + - `SkillNotFoundError` → 404 `{"error": "NOT_FOUND"}`。
  124 + - `SkillPathTraversalError` → 400 `{"error": "INVALID_SKILL_ID"}`（现实中几乎不会触发，因为
  125 + `skill_id` 已先过 DB 校验；但必须存在这条分支，不能让底层异常冒泡成 500）。
  126 + - 成功 → 200，响应体为 4.1 定义的 JSON，同时带上该 Skill 行的其余字段（`owner_org_node` 等，
  127 + 复用 `list_skills()` 已有的字段读取方式）。
  128 +
  129 +### FR-3：前端 `v_skill_audit_log`（`renderSkillAuditLog`/`mountSkillAuditLog`）
  130 +
  131 +- `renderSkillAuditLog`：在既有 `#skill_audit_list` 面板**之上或并列**新增一个“Skill 内容”面
  132 + 板，结构参照 `v_artifact_browser` 的 `.preview` 区块：
  133 + ```html
  134 + <div class="panel">
  135 + <h2>Skill 内容</h2>
  136 + <div id="skill_content_body"><div class="hint">加载中…</div></div>
  137 + </div>
  138 + ```
  139 + 不删除、不修改任何既有 `#skill_audit_error` / `#skill_audit_list` 相关 DOM。
  140 +- `mountSkillAuditLog`：新增一个 `refreshContent()`，与既有 `refresh()`（迭代记录）各自独立
  141 + 发起 fetch、各自独立渲染，二者互不阻塞（其中一个失败不影响另一个正常展示）。两者共用同一个
  142 + `#skill_audit_error` 顶部错误横幅——因为 404（Skill 完全不存在）这一失败态对两个面板都成
  143 + 立，无需拆成两条错误提示。
  144 +- 内容渲染：
  145 + ```html
  146 + <pre class="code">${content == null ? "（内容文件不可用）" : escapeHtml(content)}</pre>
  147 + ```
  148 + 与 `showPreview()` 里 `artifact.content == null ? "（文件不可用）" : escapeHtml(artifact.content)`
  149 + 完全同构，不引入任何新的渲染方式。
  150 +- 支持文件清单：`supporting_files.length` 为 0 时显示“无支持文件”提示文案；非空时渲染为纯文
  151 + 本 `<ul><li>` 列表（`escapeHtml` 处理），不加点击事件、不做预览入口。
  152 +
  153 +## 6. 安全需求（路径穿越防护，对应 BRD 要求 4）
  154 +
  155 +| 层 | 校验内容 | 失败行为 |
  156 +|---|---|---|
  157 +| DB 查询 | `skill_id` 通过参数化 SQL 查询 `skill` 表 | 查不到 → `SkillNotFoundError` → 404，不触碰文件系统 |
  158 +| 路径来源 | 文件路径必须来自 DB 的 `file_path` 列，不直接拼接原始 `skill_id` | 代码审查/测试拦截任何 `os.path.join(..., skill_id, ...)` 式写法 |
  159 +| 路径包含性 | 解析出的绝对路径必须位于 `repo_root/skills/` 之内（`realpath` + 前缀校验） | 越界 → `SkillPathTraversalError` → 400，不读取文件 |
  160 +
  161 +必须补充的测试场景（对应 §8 测试计划）：向 `skill` 表插入一条 `file_path` 被人为篡改为
  162 +`"../../etc/"` 或 `"../secrets/"` 的记录，断言 `get_skill_content` 拒绝读取并抛出
  163 +`SkillPathTraversalError`，而不是把上层目录内容读出来返回。
  164 +
  165 +## 7. 兼容性与回归范围
  166 +
  167 +- `GET /api/skills`、`GET /api/skills/:skill_id/audit`、`GET /api/skills/diff`、
  168 + `POST /api/skills/:skill_id/rollback` 四个既有接口的路由匹配顺序、响应契约、行为均不变——新
  169 + 路由是在这些后缀匹配全部落空之后才命中的新分支。
  170 +- 现有 `src/test_skill_query.py`、`src/test_backend_server.py::SkillRouteTest` 中的既有用例必
  171 + 须全部保持通过，不允许为了实现新功能而修改其断言。
  172 +- `skill_update.py` 顶部“`skills/<skill_id>/` doesn't exist as real files”这句过期注释，若实
  173 + 现时顺手核实到已不成立，可以更新措辞，但不是本次改动的必答项，不应扩大到重构
  174 + `skill_update.py` 其他逻辑。
  175 +
  176 +## 8. 测试计划
  177 +
  178 +- `src/test_skill_query.py` 新增：
  179 + - 正常路径：Skill 目录下有真实 `SKILL.md`，`get_skill_content` 返回预期 `content` 文本。
  180 + - 磁盘文件缺失：DB 有行、`SKILL.md` 不存在，`content` 为 `None`，不抛异常。
  181 + - `SkillNotFoundError`：`skill_id` 不在 `skill` 表中。
  182 + - 支持文件枚举：目录下存在 `references/`、`scripts/` 等子路径时，`supporting_files` 返回排
  183 + 序后的相对路径列表；目录下只有 `SKILL.md` 时返回空列表。
  184 + - 路径穿越拒绝：`file_path` 被篡改为越界值时抛出 `SkillPathTraversalError`，且断言底层没有
  185 + 发生对越界路径的 `open()`/读取（可用临时目录 + 越界哨兵文件验证内容未被读出）。
  186 +- `src/test_backend_server.py::SkillRouteTest` 新增：
  187 + - `GET /api/skills/skill-1` → 200，响应体含 `content`/`supporting_files` 字段。
  188 + - `GET /api/skills/no-such-skill` → 404 `NOT_FOUND`（与 `/audit` 现有 404 行为对齐）。
  189 + - 既有 `.../audit`、`.../rollback`、`/api/skills/diff` 用例路由不受新分支影响（保持通过即
  190 + 是回归验证）。
  191 +- 前端：本仓库无构建步骤、无前端单测框架（仅 `frontend/test_graph_console_smoke.js` 这一支现
  192 + 有 smoke 脚本），本次不新增前端自动化测试基建；以后端契约测试 + 手工核对
  193 + `docs/wireframes/v_skill_audit_log.html` 对应字段为验收依据，与仓库现有前端验证惯例一致。
  194 +
  195 +## 9. 验收标准（Acceptance Criteria）
  196 +
  197 +1. 打开任意已有真实 `SKILL.md` 内容的 Skill 详情页，“Skill 内容”面板显示该文件的真实全文文
  198 + 本（纯 `<pre>`，无 Markdown 渲染）。
  199 +2. 该 Skill 的“变更审计”面板行为与改动前完全一致（空态文案、迭代列表、diff、rollback 均不
  200 + 变）。
  201 +3. 两个面板同时展示、互不覆盖、互不依赖对方是否为空。
  202 +4. 磁盘 `SKILL.md` 缺失但 DB 中有该 Skill 记录时，内容面板显示“（内容文件不可用）”，不报
  203 + 500，不使页面崩溃。
  204 +5. `skill_id` 不存在时，整页显示“该 Skill 不存在”错误态（两个面板都不渲染），接口返回 404。
  205 +6. 构造 `file_path` 越界的 Skill 记录，接口不会读出 `skills/` 目录之外的任意文件内容，返回
  206 + 400，不是 500，也不是把越界内容当成功返回。
  207 +7. `src/test_skill_query.py`、`src/test_backend_server.py` 全量既有用例 + 本次新增用例全部通
  208 + 过；未修改任何既有断言。
  209 +8. 前端未引入任何新的第三方依赖（无 `<script src="...markdown...">` 之类的新增外部库引用）。