From 0b5f707e62821c71c88dbbb39afa200ffb31f48f Mon Sep 17 00:00:00 2001 From: DYD Date: Sun, 3 May 2026 20:01:18 +0800 Subject: [PATCH 1/6] feat: add git-backed governance adapter --- docs/modules/02-skill-governance.md | 244 ++++++++---------- .../layers/skill_governance/__init__.py | 4 + .../skill_governance/git_version_store.py | 196 ++++++++++++++ .../test_skill_governance_git_adapter.py | 87 +++++++ 4 files changed, 390 insertions(+), 141 deletions(-) create mode 100644 skillos/skillos/layers/skill_governance/git_version_store.py create mode 100644 skillos/tests/test_skill_governance_git_adapter.py diff --git a/docs/modules/02-skill-governance.md b/docs/modules/02-skill-governance.md index fab9370..fe613f5 100644 --- a/docs/modules/02-skill-governance.md +++ b/docs/modules/02-skill-governance.md @@ -1,179 +1,141 @@ # Module 02: Skill Governance Layer -**负责人分支：`governance-dev`** +**负责分支：`governance-dev`** --- -## 职责概述 +## 职责概览 -Skill Governance Layer 负责 Skill 的全生命周期治理，包括： -- Git 式版本控制（branch/commit/PR/review/merge） -- 状态机驱动的生命周期管理（S0-S7） -- Skill 构建与验证（从候选到草稿） -- 变更历史与 diff 视图 +Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包括： + +- Skill 生命周期状态流转：S0-S7。 +- Skill 审核、发布、废弃、修复后的治理记录。 +- Skill 版本历史、diff、breaking change 判断。 +- 基于 Git 的 branch / commit / history / diff / rollback / PR 工作流封装。 + +当前方向不是重新实现一个 Git-like 系统，而是把 Git 作为底层版本事实来源。SkillOS 只在 Git 之上增加 Skill 语义，例如“某次 commit 对应哪个 Skill 版本”“某个 diff 是否是 breaking change”“某次修复是否需要 review”。 --- -## 子模块 +## 当前子模块 -### 2.1 Version Control（`layers/skill_governance/version_control.py`） +### 2.1 Version Control -实现 Git 式版本管理，每次 `new-version` 操作创建新的 Skill 实例（不可变版本）。 +`layers/skill_governance/version_control.py` -**版本号规则（Semantic Versioning）：** -- `patch`：1.0.0 → 1.0.1（bug 修复、小调整） -- `minor`：1.0.0 → 1.1.0（新功能、向后兼容） -- `major`：1.0.0 → 2.0.0（破坏性变更） +保留现有语义化版本控制能力： -**变更记录（ChangeRecord）：** -```python -@dataclass -class ChangeRecord: - record_id: str - skill_id: str - from_version: str - to_version: str - change_type: str # "patch" / "minor" / "major" - summary: str - author: str - created_at: datetime - diff: Dict[str, Any] # {field: {old: ..., new: ...}} - is_breaking: bool -``` +- 记录 `ChangeRecord`。 +- 计算两个 Skill 对象之间的字段级 diff。 +- 根据 diff 建议 `major` / `minor` / `patch`。 +- 创建新版本时把 Skill 回到 Draft 状态，等待后续审核。 -### 2.2 Lifecycle Management（`api/routes/lifecycle.py`） +### 2.2 Git Version Store -状态转换的 API 层，封装了所有合法的状态迁移操作。 +`layers/skill_governance/git_version_store.py` -**合法转换路径：** -``` -S0 → S1 (ingest) -S1 → S2 (review) -S2 → S3 (verify/audit) -S3 → S4 (release) -S4 → S5 (degrade, 自动触发) -S5 → S4 (repair) -S4/S5 → S6 (deprecate) -S6 → S7 (archive) -``` +第一阶段新增的 Git 包装层，职责是安全调用 Git，而不是实现 Git： -**Skill 模型上的状态方法：** -```python -class Skill: - def transition_to(self, new_state: SkillState, reason: str = "") - def record_execution(self, success: bool, latency_ms: float) - # 自动降级：success_rate < 0.6 且 total_executions >= 10 → S5 -``` +- 检查目标目录是否是 Git 仓库。 +- 获取当前分支。 +- 获取当前 HEAD commit。 +- 获取某个 Skill 快照文件的 commit history。 +- 获取两个 commit 之间的 Git diff。 +- 在测试用临时仓库中创建 commit。 -### 2.3 Reviewer（`layers/skill_governance/reviewer.py`） +这一层暂时不直接改 lifecycle API，也不自动写入真实 Skill 快照。后续阶段会把 Skill JSON 快照接到这一层。 -LLM 驱动的 Skill 审核，检查描述完整性、接口合理性、实现安全性。 +### 2.3 Lifecycle Management -### 2.4 Merger（`layers/skill_governance/merger.py`） +`api/routes/lifecycle.py` -将功能重复的多个 Skill 合并为一个统一版本。 +封装现有生命周期接口： -### 2.5 Skill Construction（`layers/skill_construction/`） +| 方法 | 路径 | 功能 | +| --- | --- | --- | +| `POST` | `/api/v1/lifecycle/{id}/transition` | 手动状态流转 | +| `POST` | `/api/v1/lifecycle/{id}/release` | 发布 Skill | +| `POST` | `/api/v1/lifecycle/{id}/deprecate` | 废弃 Skill | +| `POST` | `/api/v1/lifecycle/{id}/new-version` | 创建新版本 | +| `POST` | `/api/v1/lifecycle/{id}/review` | LLM 审核 | +| `POST` | `/api/v1/lifecycle/{id}/review-and-release` | 审核并发布 | +| `POST` | `/api/v1/lifecycle/{id}/record-execution` | 记录执行结果 | +| `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史 diff | +| `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个版本 | -从候选经验构建正式 Skill： +第一阶段不修改这些接口的请求和响应字段。 -| 文件 | 职责 | -|------|------| -| `candidate_miner.py` | 从经验单元中挖掘 Skill 候选 | -| `formalizer.py` | 将非正式描述规范化为标准 JSON Schema | -| `validator.py` | 验证 Skill 定义的完整性和合法性 | +### 2.4 Reviewer / Merger ---- +`layers/skill_governance/reviewer.py` -## 工作流详解 - -### 新版本创建流程 - -``` -POST /lifecycle/{id}/new-version { bump: "patch" } - │ - ▼ -1. 获取当前 Skill（必须是 S4 Released） -2. 计算新版本号（semver bump） -3. 克隆 Skill，更新 version、state=S2 Draft -4. 记录 ChangeRecord（diff = 空，等待后续修改） -5. 写入 SkillWiki（新 skill_id） -6. 返回新版本 Skill -``` - -### Diff 计算流程 - -``` -GET /lifecycle/{id}/diff - │ - ▼ -1. 获取该 Skill 的所有版本历史 -2. 对相邻版本计算字段级 diff -3. _format_diff() 将 {field: {old, new}} 转换为 - [{field, type, old_lines, new_lines}] 格式 -4. 返回完整变更历史列表 -``` - -### 自动降级流程 - -``` -每次 record_execution() 调用后： - │ - ▼ -计算 success_rate = successful / total - │ - ├── success_rate < 0.6 AND total >= 10 AND state == S4 - │ → 自动 transition_to(S5, "自动降级：成功率过低") - │ - └── 否则保持当前状态 -``` +`layers/skill_governance/merger.py` + +现有 Reviewer 负责 Skill 质量审核，Merger 负责相似 Skill 合并和大 Skill 拆分。B 后续阶段会让这些治理动作也产生 Git-backed 版本记录。 --- -## API 端点 +## B 任务阶段规划 -| 方法 | 路径 | 功能 | -|------|------|------| -| `POST` | `/api/v1/lifecycle/{id}/transition` | 手动状态转换 | -| `POST` | `/api/v1/lifecycle/{id}/release` | 发布（S2/S3 → S4） | -| `POST` | `/api/v1/lifecycle/{id}/deprecate` | 废弃（→ S6） | -| `POST` | `/api/v1/lifecycle/{id}/new-version` | 创建新版本 | -| `POST` | `/api/v1/lifecycle/{id}/review` | LLM 审核 | -| `POST` | `/api/v1/lifecycle/{id}/review-and-release` | 审核并发布 | -| `POST` | `/api/v1/lifecycle/{id}/record-execution` | 记录执行结果 | -| `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史与 diff | -| `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个特定版本 | +### 阶段一：Git-backed Version Adapter 底座 ---- +目标：先把 Git 调用能力放进 governance 层，保证后续不用自研 Git-like 版本系统。 -## 关键文件 - -``` -skillos/skillos/ -├── layers/ -│ ├── skill_governance/ -│ │ ├── version_control.py # 版本控制核心逻辑 -│ │ ├── reviewer.py # LLM 审核 -│ │ └── merger.py # Skill 合并 -│ └── skill_construction/ -│ ├── candidate_miner.py # 候选挖掘 -│ ├── formalizer.py # Schema 规范化 -│ └── validator.py # 合法性验证 -├── models/skill_model.py # SkillState 枚举、transition_to() -└── api/routes/lifecycle.py # 生命周期 API 路由 -``` +完成内容： ---- +- 新增 `GitVersionStore`。 +- 新增临时 Git 仓库测试。 +- 保持 REST 接口不变。 +- 更新本模块文档，明确“Git 外壳”路线。 -## 优化方向（Member B 任务） +### 阶段二：Skill 快照与领域级 Diff -1. **Diff 精细化**：当前 diff 只比较顶层字段，可深入比较 `interface.input_schema` 的具体字段变化 -2. **Breaking Change 检测**：自动检测 major bump 是否真的有破坏性变更（如删除必填输入字段） -3. **审核流程完善**：`reviewer.py` 当前是 LLM 调用，可增加规则引擎（如检查 prompt_template 是否包含所有 input_schema 字段） -4. **版本回滚**：增加 `POST /lifecycle/{id}/rollback/{version}` 端点 -5. **变更通知**：当 Skill 状态变更时，通过 WebSocket 广播事件（当前已有 ws.py 基础设施） -6. **Skill Construction 完善**：`candidate_miner.py` 和 `formalizer.py` 的 LLM prompt 可进一步优化 +目标：把 Skill 序列化为稳定 JSON 快照，并用 Git diff + Skill 字段级解释生成更可读的版本差异。 + +重点： + +- Skill snapshot 路径规则。 +- JSON 稳定排序。 +- interface / implementation / prompt_template 的字段级 diff。 +- breaking change 检测。 + +### 阶段三：Branch / Review 工作流封装 + +目标：把 Skill 修改映射为 Git branch 和 review 流程。 + +重点： + +- Skill 修改分支。 +- commit message 规范。 +- review 状态与 lifecycle 状态对应。 +- 后续可接 GitHub PR，但不自造 PR 系统。 + +### 阶段四：Rollback / Tag / Release + +目标：支持按 commit 或版本 tag 回滚 Skill。 + +重点： + +- version tag 规则。 +- rollback API 设计。 +- rollback 后的 Skill 状态与审计记录。 + +### 阶段五：联调与交付 + +目标：与 E 前端和 D 自管理 Agent 联调。 + +重点： + +- E 展示 Skill 版本历史和 diff。 +- D 的 repair / merge / split 产生可追踪版本记录。 +- 中文 PR 说明、交付文档、组长 review。 --- -*更新此文档时请同步更新 `architecture.md` 中的 Governance Flow 部分（联系负责人）* +## 当前边界 + +- 不修改 `docs/interfaces.md` 和 `docs/architecture.md`。 +- 不新增依赖。 +- 不改现有 lifecycle API 字段。 +- 不碰 E 的 `frontend-dev` PR。 +- 不碰 D 的 `agents-dev` PR。 diff --git a/skillos/skillos/layers/skill_governance/__init__.py b/skillos/skillos/layers/skill_governance/__init__.py index 483c771..161f801 100644 --- a/skillos/skillos/layers/skill_governance/__init__.py +++ b/skillos/skillos/layers/skill_governance/__init__.py @@ -1,10 +1,14 @@ """skill_governance 层包导出。""" +from .git_version_store import GitCommit, GitVersionStore, GitVersionStoreError from .merger import MergeResult, SkillMerger, SplitResult from .reviewer import ReviewResult, ReviewStatus, SkillReviewer from .version_control import ChangeRecord, ChangeType, VersionController __all__ = [ + "GitVersionStore", + "GitVersionStoreError", + "GitCommit", "VersionController", "ChangeRecord", "ChangeType", diff --git a/skillos/skillos/layers/skill_governance/git_version_store.py b/skillos/skillos/layers/skill_governance/git_version_store.py new file mode 100644 index 0000000..e0ccd04 --- /dev/null +++ b/skillos/skillos/layers/skill_governance/git_version_store.py @@ -0,0 +1,196 @@ +"""Git-backed version store for Skill governance. + +Git remains the source of truth for branches, commits, history, and diffs. +SkillOS adds Skill-level meaning above this adapter in later stages. +""" + +from __future__ import annotations + +import subprocess +from dataclasses import dataclass +from pathlib import Path +from typing import List, Optional, Sequence, Tuple + + +class GitVersionStoreError(RuntimeError): + """Raised when a Git-backed version operation cannot be completed.""" + + +@dataclass(frozen=True) +class GitCommit: + """Small, JSON-friendly commit summary.""" + + commit_hash: str + author: str + authored_at: str + subject: str + changed_paths: Tuple[str, ...] = () + + +class GitVersionStore: + """Thin wrapper around Git commands used by the governance layer.""" + + def __init__(self, repo_path: str | Path, timeout_seconds: float = 10.0) -> None: + self.repo_path = Path(repo_path) + self.timeout_seconds = timeout_seconds + + def is_git_repo(self) -> bool: + """Return True when repo_path is inside a Git work tree.""" + try: + output = self._run(["rev-parse", "--is-inside-work-tree"], check=False) + except GitVersionStoreError: + return False + return output.strip().lower() == "true" + + def current_branch(self) -> str: + """Return the current branch name, or HEAD for detached checkouts.""" + self._require_repo() + return self._run(["rev-parse", "--abbrev-ref", "HEAD"]).strip() + + def head_commit(self) -> str: + """Return the current HEAD commit hash.""" + self._require_repo() + return self._run(["rev-parse", "HEAD"]).strip() + + def commit_paths( + self, + paths: Sequence[str | Path], + message: str, + author_name: str = "SkillOS", + author_email: str = "skillos@example.local", + ) -> str: + """Commit selected repo-relative paths and return the new HEAD hash.""" + self._require_repo() + normalized_paths = [self._normalize_repo_path(path) for path in paths] + if not normalized_paths: + raise ValueError("At least one path is required to create a commit.") + if not message.strip(): + raise ValueError("Commit message cannot be empty.") + + self._run(["add", "--", *normalized_paths]) + self._run( + [ + "-c", + f"user.name={author_name}", + "-c", + f"user.email={author_email}", + "commit", + "-m", + message, + "--", + *normalized_paths, + ] + ) + return self.head_commit() + + def commit_history(self, path: str | Path, max_count: int = 20) -> List[GitCommit]: + """Return newest-first commit history for a repo-relative path.""" + self._require_repo() + if max_count <= 0: + raise ValueError("max_count must be greater than zero.") + + repo_path = self._normalize_repo_path(path) + marker = "--SKILLOS-COMMIT--" + fmt = f"{marker}%x1f%H%x1f%an%x1f%aI%x1f%s" + output = self._run( + [ + "log", + f"--max-count={max_count}", + f"--format={fmt}", + "--name-only", + "--", + repo_path, + ] + ) + return self._parse_history(output, marker) + + def diff_between( + self, + from_ref: str, + to_ref: str, + path: Optional[str | Path] = None, + ) -> str: + """Return a unified, no-color Git diff between two refs.""" + self._require_repo() + args = ["diff", "--no-color", "--no-ext-diff", f"{from_ref}..{to_ref}"] + if path is not None: + args.extend(["--", self._normalize_repo_path(path)]) + return self._run(args) + + def _require_repo(self) -> None: + if not self.is_git_repo(): + raise GitVersionStoreError(f"{self.repo_path} is not a Git repository.") + + def _run(self, args: Sequence[str], check: bool = True) -> str: + command = ["git", *args] + try: + completed = subprocess.run( + command, + cwd=self.repo_path, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=self.timeout_seconds, + ) + except FileNotFoundError as exc: + raise GitVersionStoreError("Git executable was not found.") from exc + except subprocess.TimeoutExpired as exc: + printable = " ".join(command) + raise GitVersionStoreError(f"Git command timed out: {printable}") from exc + + if check and completed.returncode != 0: + printable = " ".join(command) + detail = completed.stderr.strip() or completed.stdout.strip() or "unknown error" + raise GitVersionStoreError(f"Git command failed: {printable}: {detail}") + return completed.stdout + + @staticmethod + def _normalize_repo_path(path: str | Path) -> str: + raw = str(path).replace("\\", "/").strip() + if not raw: + raise ValueError("Git path cannot be empty.") + parts = [part for part in raw.split("/") if part not in ("", ".")] + if Path(raw).is_absolute() or ".." in parts: + raise ValueError("Git path must be repo-relative and stay inside the repository.") + return "/".join(parts) + + @staticmethod + def _parse_history(output: str, marker: str) -> List[GitCommit]: + commits: List[GitCommit] = [] + current: Optional[GitCommit] = None + changed_paths: List[str] = [] + + def flush() -> None: + nonlocal current, changed_paths + if current is None: + return + commits.append( + GitCommit( + commit_hash=current.commit_hash, + author=current.author, + authored_at=current.authored_at, + subject=current.subject, + changed_paths=tuple(changed_paths), + ) + ) + current = None + changed_paths = [] + + for line in output.splitlines(): + if line.startswith(marker): + flush() + parts = line.split("\x1f", 4) + if len(parts) != 5: + continue + current = GitCommit( + commit_hash=parts[1], + author=parts[2], + authored_at=parts[3], + subject=parts[4], + ) + elif current is not None and line.strip(): + changed_paths.append(line.strip()) + + flush() + return commits diff --git a/skillos/tests/test_skill_governance_git_adapter.py b/skillos/tests/test_skill_governance_git_adapter.py new file mode 100644 index 0000000..10f46fb --- /dev/null +++ b/skillos/tests/test_skill_governance_git_adapter.py @@ -0,0 +1,87 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from skillos.layers.skill_governance import GitVersionStore, GitVersionStoreError + + +def _run_git(repo: Path, args: list[str]) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +@pytest.fixture +def git_repo(tmp_path: Path) -> Path: + _run_git(tmp_path, ["init"]) + return tmp_path + + +def test_detects_git_repo_and_head(git_repo: Path) -> None: + skill_path = git_repo / "skills" / "test_skill.json" + skill_path.parent.mkdir() + skill_path.write_text('{"name": "test_skill", "version": "1.0.0"}\n', encoding="utf-8") + + store = GitVersionStore(git_repo) + commit_hash = store.commit_paths(["skills/test_skill.json"], "add test skill") + + assert store.is_git_repo() + assert store.current_branch() + assert store.head_commit() == commit_hash + assert len(commit_hash) == 40 + + +def test_commit_history_for_skill_snapshot(git_repo: Path) -> None: + skill_path = git_repo / "skills" / "test_skill.json" + skill_path.parent.mkdir() + skill_path.write_text('{"version": "1.0.0"}\n', encoding="utf-8") + + store = GitVersionStore(git_repo) + store.commit_paths(["skills/test_skill.json"], "add test skill") + skill_path.write_text('{"version": "1.0.1"}\n', encoding="utf-8") + store.commit_paths(["skills/test_skill.json"], "update test skill") + + history = store.commit_history("skills/test_skill.json") + + assert [entry.subject for entry in history] == ["update test skill", "add test skill"] + assert history[0].changed_paths == ("skills/test_skill.json",) + + +def test_diff_between_commits(git_repo: Path) -> None: + skill_path = git_repo / "skills" / "test_skill.json" + skill_path.parent.mkdir() + skill_path.write_text('{"version": "1.0.0"}\n', encoding="utf-8") + + store = GitVersionStore(git_repo) + first_commit = store.commit_paths(["skills/test_skill.json"], "add test skill") + skill_path.write_text('{"version": "1.0.1"}\n', encoding="utf-8") + second_commit = store.commit_paths(["skills/test_skill.json"], "update test skill") + + diff = store.diff_between(first_commit, second_commit, "skills/test_skill.json") + + assert '"1.0.0"' in diff + assert '"1.0.1"' in diff + + +def test_non_git_repo_reports_clear_error(tmp_path: Path) -> None: + store = GitVersionStore(tmp_path) + + assert not store.is_git_repo() + with pytest.raises(GitVersionStoreError, match="not a Git repository"): + store.current_branch() + + +def test_rejects_paths_outside_repo(git_repo: Path) -> None: + store = GitVersionStore(git_repo) + + with pytest.raises(ValueError, match="repo-relative"): + store.commit_paths(["../outside.json"], "invalid path") From 6be5aa136d5683e0363cca75cd04b42f0f5aea3e Mon Sep 17 00:00:00 2001 From: DYD Date: Sun, 3 May 2026 20:19:34 +0800 Subject: [PATCH 2/6] feat: add skill snapshot diff --- docs/modules/02-skill-governance.md | 42 +++- .../layers/skill_governance/__init__.py | 20 ++ .../layers/skill_governance/skill_snapshot.py | 210 ++++++++++++++++ .../test_skill_governance_snapshot_diff.py | 225 ++++++++++++++++++ 4 files changed, 487 insertions(+), 10 deletions(-) create mode 100644 skillos/skillos/layers/skill_governance/skill_snapshot.py create mode 100644 skillos/tests/test_skill_governance_snapshot_diff.py diff --git a/docs/modules/02-skill-governance.md b/docs/modules/02-skill-governance.md index fe613f5..3735ce4 100644 --- a/docs/modules/02-skill-governance.md +++ b/docs/modules/02-skill-governance.md @@ -43,9 +43,29 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 - 获取两个 commit 之间的 Git diff。 - 在测试用临时仓库中创建 commit。 -这一层暂时不直接改 lifecycle API，也不自动写入真实 Skill 快照。后续阶段会把 Skill JSON 快照接到这一层。 +### 2.3 Skill Snapshot / Domain Diff -### 2.3 Lifecycle Management +`layers/skill_governance/skill_snapshot.py` + +第二阶段新增 Skill 语义层能力： + +- 将 `Skill` 稳定序列化为 JSON 快照。 +- 快照路径固定为 `skills//.json`。 +- 快照排除 `metrics` 和时间戳字段，避免运行时噪声污染版本 diff。 +- 支持写入临时或指定 Git 仓库，并复用 `GitVersionStore` 生成 commit。 +- 支持字段级 diff 和 breaking change 检测。 + +当前 breaking change 规则： + +- 删除 input schema property。 +- 给 input schema 新增 required 字段。 +- 删除 output schema property。 +- 修改已有 schema property 的 `type`。 +- 删除或清空已有 `implementation.prompt_template` / `implementation.code`。 + +这些能力暂时不直接改 lifecycle API，也不自动写入真实项目仓库中的 Skill 快照。后续阶段会把它接入 branch / review / rollback 工作流。 + +### 2.4 Lifecycle Management `api/routes/lifecycle.py` @@ -63,9 +83,9 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 | `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史 diff | | `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个版本 | -第一阶段不修改这些接口的请求和响应字段。 +前两阶段均不修改这些接口的请求和响应字段。 -### 2.4 Reviewer / Merger +### 2.5 Reviewer / Merger `layers/skill_governance/reviewer.py` @@ -81,7 +101,7 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 目标：先把 Git 调用能力放进 governance 层，保证后续不用自研 Git-like 版本系统。 -完成内容： +已完成： - 新增 `GitVersionStore`。 - 新增临时 Git 仓库测试。 @@ -92,12 +112,14 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 目标：把 Skill 序列化为稳定 JSON 快照，并用 Git diff + Skill 字段级解释生成更可读的版本差异。 -重点： +已完成： -- Skill snapshot 路径规则。 -- JSON 稳定排序。 -- interface / implementation / prompt_template 的字段级 diff。 -- breaking change 检测。 +- 新增 Skill snapshot 稳定序列化。 +- 约定 snapshot 路径规则。 +- 排除运行时噪声字段。 +- 新增领域级 diff。 +- 新增 breaking change 检测。 +- 新增临时 Git 仓库快照提交测试。 ### 阶段三：Branch / Review 工作流封装 diff --git a/skillos/skillos/layers/skill_governance/__init__.py b/skillos/skillos/layers/skill_governance/__init__.py index 161f801..58e184d 100644 --- a/skillos/skillos/layers/skill_governance/__init__.py +++ b/skillos/skillos/layers/skill_governance/__init__.py @@ -3,12 +3,32 @@ from .git_version_store import GitCommit, GitVersionStore, GitVersionStoreError from .merger import MergeResult, SkillMerger, SplitResult from .reviewer import ReviewResult, ReviewStatus, SkillReviewer +from .skill_snapshot import ( + SkillSnapshotDiff, + commit_skill_snapshot, + diff_skill_snapshots, + has_breaking_changes, + skill_snapshot_path, + skill_to_snapshot, + skill_to_snapshot_json, + snapshot_to_json, + write_skill_snapshot, +) from .version_control import ChangeRecord, ChangeType, VersionController __all__ = [ "GitVersionStore", "GitVersionStoreError", "GitCommit", + "SkillSnapshotDiff", + "skill_snapshot_path", + "skill_to_snapshot", + "snapshot_to_json", + "skill_to_snapshot_json", + "write_skill_snapshot", + "commit_skill_snapshot", + "diff_skill_snapshots", + "has_breaking_changes", "VersionController", "ChangeRecord", "ChangeType", diff --git a/skillos/skillos/layers/skill_governance/skill_snapshot.py b/skillos/skillos/layers/skill_governance/skill_snapshot.py new file mode 100644 index 0000000..85f50c1 --- /dev/null +++ b/skillos/skillos/layers/skill_governance/skill_snapshot.py @@ -0,0 +1,210 @@ +"""Skill snapshot serialization and domain-level diff utilities.""" + +from __future__ import annotations + +import json +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Dict, List, Mapping, Optional + +from ...models.skill_model import Skill +from .git_version_store import GitVersionStore + + +SNAPSHOT_FIELDS = ( + "skill_id", + "name", + "version", + "description", + "skill_type", + "domain", + "granularity_level", + "state", + "tags", + "interface", + "implementation", + "test_cases", + "provenance", +) + +DIFF_FIELDS = ( + "name", + "version", + "description", + "skill_type", + "domain", + "state", + "tags", + "interface.input_schema", + "interface.output_schema", + "implementation.prompt_template", + "implementation.code", + "implementation.tool_calls", + "implementation.sub_skill_ids", +) + + +@dataclass(frozen=True) +class SkillSnapshotDiff: + """A single field-level snapshot diff.""" + + field: str + change_type: str + old_value: Any + new_value: Any + is_breaking: bool = False + + def to_dict(self) -> Dict[str, Any]: + return { + "field": self.field, + "change_type": self.change_type, + "old_value": self.old_value, + "new_value": self.new_value, + "is_breaking": self.is_breaking, + } + + +def skill_snapshot_path(skill: Skill) -> str: + """Return the repo-relative snapshot path for a Skill version.""" + return f"skills/{skill.skill_id}/{skill.version}.json" + + +def skill_to_snapshot(skill: Skill) -> Dict[str, Any]: + """Convert a Skill to a stable governance snapshot dictionary.""" + raw = skill.model_dump(mode="json") + return {field: raw.get(field) for field in SNAPSHOT_FIELDS} + + +def snapshot_to_json(snapshot: Mapping[str, Any]) -> str: + """Serialize a snapshot to stable JSON.""" + return json.dumps(snapshot, ensure_ascii=False, sort_keys=True, indent=2) + "\n" + + +def skill_to_snapshot_json(skill: Skill) -> str: + """Serialize a Skill to stable governance snapshot JSON.""" + return snapshot_to_json(skill_to_snapshot(skill)) + + +def write_skill_snapshot(repo_path: str | Path, skill: Skill) -> str: + """Write a Skill snapshot under the repo path and return its relative path.""" + relative_path = skill_snapshot_path(skill) + target = Path(repo_path) / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(skill_to_snapshot_json(skill), encoding="utf-8") + return relative_path + + +def commit_skill_snapshot( + repo_path: str | Path, + skill: Skill, + store: Optional[GitVersionStore] = None, +) -> str: + """Write and commit a Skill snapshot, returning the new commit hash.""" + relative_path = write_skill_snapshot(repo_path, skill) + version_store = store or GitVersionStore(repo_path) + message = f"skill({skill.name}): snapshot v{skill.version}" + return version_store.commit_paths([relative_path], message) + + +def diff_skill_snapshots( + old_snapshot: Mapping[str, Any] | str, + new_snapshot: Mapping[str, Any] | str, +) -> List[SkillSnapshotDiff]: + """Return domain-level diffs between two Skill snapshots.""" + old_data = _coerce_snapshot(old_snapshot) + new_data = _coerce_snapshot(new_snapshot) + diffs: List[SkillSnapshotDiff] = [] + + for field in DIFF_FIELDS: + old_value = _get_path(old_data, field) + new_value = _get_path(new_data, field) + if old_value == new_value: + continue + diffs.extend(_diff_field(field, old_value, new_value)) + + return diffs + + +def has_breaking_changes(diffs: List[SkillSnapshotDiff]) -> bool: + return any(diff.is_breaking for diff in diffs) + + +def _coerce_snapshot(snapshot: Mapping[str, Any] | str) -> Dict[str, Any]: + if isinstance(snapshot, str): + return json.loads(snapshot) + return dict(snapshot) + + +def _get_path(data: Mapping[str, Any], path: str) -> Any: + current: Any = data + for part in path.split("."): + if not isinstance(current, Mapping) or part not in current: + return None + current = current[part] + return current + + +def _diff_field(field: str, old_value: Any, new_value: Any) -> List[SkillSnapshotDiff]: + if field in ("interface.input_schema", "interface.output_schema"): + return _diff_schema(field, old_value or {}, new_value or {}) + + change_type = _change_type(old_value, new_value) + is_breaking = field in ("implementation.prompt_template", "implementation.code") and ( + bool(old_value) and not new_value + ) + return [SkillSnapshotDiff(field, change_type, old_value, new_value, is_breaking)] + + +def _diff_schema( + field: str, + old_schema: Mapping[str, Any], + new_schema: Mapping[str, Any], +) -> List[SkillSnapshotDiff]: + diffs: List[SkillSnapshotDiff] = [] + old_properties = old_schema.get("properties", {}) if isinstance(old_schema, Mapping) else {} + new_properties = new_schema.get("properties", {}) if isinstance(new_schema, Mapping) else {} + old_required = set(old_schema.get("required", []) if isinstance(old_schema, Mapping) else []) + new_required = set(new_schema.get("required", []) if isinstance(new_schema, Mapping) else []) + + for name in sorted(set(old_properties) | set(new_properties)): + old_prop = old_properties.get(name) + new_prop = new_properties.get(name) + if old_prop == new_prop: + continue + path = f"{field}.properties.{name}" + if name not in new_properties: + diffs.append( + SkillSnapshotDiff( + path, + "removed", + old_prop, + None, + field in ("interface.input_schema", "interface.output_schema"), + ) + ) + elif name not in old_properties: + diffs.append(SkillSnapshotDiff(path, "added", None, new_prop, False)) + else: + old_type = old_prop.get("type") if isinstance(old_prop, Mapping) else None + new_type = new_prop.get("type") if isinstance(new_prop, Mapping) else None + diffs.append( + SkillSnapshotDiff(path, "modified", old_prop, new_prop, old_type != new_type) + ) + + for name in sorted(old_required - new_required): + path = f"{field}.required.{name}" + diffs.append(SkillSnapshotDiff(path, "removed", True, False, False)) + + for name in sorted(new_required - old_required): + path = f"{field}.required.{name}" + diffs.append(SkillSnapshotDiff(path, "added", False, True, field == "interface.input_schema")) + + return diffs + + +def _change_type(old_value: Any, new_value: Any) -> str: + if old_value is None and new_value is not None: + return "added" + if old_value is not None and new_value is None: + return "removed" + return "modified" diff --git a/skillos/tests/test_skill_governance_snapshot_diff.py b/skillos/tests/test_skill_governance_snapshot_diff.py new file mode 100644 index 0000000..7ca4f0d --- /dev/null +++ b/skillos/tests/test_skill_governance_snapshot_diff.py @@ -0,0 +1,225 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +from skillos.layers.skill_governance import ( + GitVersionStore, + commit_skill_snapshot, + diff_skill_snapshots, + has_breaking_changes, + skill_snapshot_path, + skill_to_snapshot, + skill_to_snapshot_json, +) +from skillos.models import Skill, SkillImplementation, SkillInterface, SkillState, SkillType + + +def _run_git(repo: Path, args: list[str]) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +def _make_skill() -> Skill: + return Skill( + skill_id="skill-123", + name="search_wiki", + version="1.0.0", + description="Search wiki entries.", + skill_type=SkillType.FUNCTIONAL, + domain="wiki", + state=SkillState.RELEASED, + tags=["wiki", "search"], + interface=SkillInterface( + input_schema={ + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + output_schema={ + "type": "object", + "properties": {"results": {"type": "array"}}, + }, + ), + implementation=SkillImplementation(prompt_template="Search for {query}"), + ) + + +def _snapshot_with( + *, + input_schema: dict | None = None, + output_schema: dict | None = None, + description: str = "Search wiki entries.", + tags: list[str] | None = None, + implementation: dict | None = None, +) -> dict: + snapshot = skill_to_snapshot(_make_skill()) + snapshot["description"] = description + snapshot["tags"] = tags if tags is not None else ["wiki", "search"] + if input_schema is not None: + snapshot["interface"]["input_schema"] = input_schema + if output_schema is not None: + snapshot["interface"]["output_schema"] = output_schema + if implementation is not None: + snapshot["implementation"] = implementation + return snapshot + + +def test_snapshot_json_is_stable() -> None: + skill = _make_skill() + + assert skill_to_snapshot_json(skill) == skill_to_snapshot_json(skill) + + +def test_snapshot_excludes_runtime_noise_fields() -> None: + skill = _make_skill() + skill.record_execution(success=True, latency_ms=120) + snapshot = skill_to_snapshot(skill) + json_text = skill_to_snapshot_json(skill) + + assert "metrics" not in snapshot + assert "created_at" not in snapshot + assert "updated_at" not in snapshot + assert "released_at" not in snapshot + assert "deprecated_at" not in snapshot + assert "metrics" not in json_text + + +def test_snapshot_path_uses_skill_id_and_version() -> None: + skill = _make_skill() + + assert skill_snapshot_path(skill) == "skills/skill-123/1.0.0.json" + + +def test_description_change_is_not_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with(description="Search wiki entries with filters.") + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert [diff.field for diff in diffs] == ["description"] + assert not has_breaking_changes(diffs) + + +def test_added_optional_input_field_is_not_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with( + input_schema={ + "type": "object", + "properties": { + "query": {"type": "string"}, + "limit": {"type": "integer"}, + }, + "required": ["query"], + } + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "interface.input_schema.properties.limit" for diff in diffs) + assert not has_breaking_changes(diffs) + + +def test_removed_input_property_is_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with( + input_schema={"type": "object", "properties": {}, "required": []} + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "interface.input_schema.properties.query" for diff in diffs) + assert has_breaking_changes(diffs) + + +def test_added_required_input_field_is_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with( + input_schema={ + "type": "object", + "properties": { + "query": {"type": "string"}, + "workspace": {"type": "string"}, + }, + "required": ["query", "workspace"], + } + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "interface.input_schema.required.workspace" for diff in diffs) + assert has_breaking_changes(diffs) + + +def test_removed_output_property_is_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with( + output_schema={"type": "object", "properties": {}} + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "interface.output_schema.properties.results" for diff in diffs) + assert has_breaking_changes(diffs) + + +def test_schema_type_change_is_breaking() -> None: + old_snapshot = _snapshot_with() + new_snapshot = _snapshot_with( + input_schema={ + "type": "object", + "properties": {"query": {"type": "array"}}, + "required": ["query"], + } + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "interface.input_schema.properties.query" for diff in diffs) + assert has_breaking_changes(diffs) + + +def test_prompt_or_code_removed_is_breaking() -> None: + old_snapshot = _snapshot_with( + implementation={"language": "python", "prompt_template": "Search {query}"} + ) + new_snapshot = _snapshot_with( + implementation={"language": "python", "prompt_template": ""} + ) + + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + + assert any(diff.field == "implementation.prompt_template" for diff in diffs) + assert has_breaking_changes(diffs) + + +def test_diff_accepts_json_strings() -> None: + old_json = skill_to_snapshot_json(_make_skill()) + new_snapshot = _snapshot_with(description="Updated description.") + + diffs = diff_skill_snapshots(old_json, new_snapshot) + + assert [diff.to_dict()["field"] for diff in diffs] == ["description"] + + +def test_commit_snapshot_to_temp_git_repo(tmp_path: Path) -> None: + _run_git(tmp_path, ["init"]) + skill = _make_skill() + store = GitVersionStore(tmp_path) + baseline = tmp_path / ".gitkeep" + baseline.write_text("", encoding="utf-8") + baseline_commit = store.commit_paths([".gitkeep"], "initial baseline") + + commit_hash = commit_skill_snapshot(tmp_path, skill, store) + history = store.commit_history(skill_snapshot_path(skill)) + diff = store.diff_between(baseline_commit, commit_hash, skill_snapshot_path(skill)) + + assert history[0].subject == "skill(search_wiki): snapshot v1.0.0" + assert "search_wiki" in diff From dbf637d388c15e1bf67c495ee6e8a8ab52ae9bb6 Mon Sep 17 00:00:00 2001 From: DYD Date: Sun, 3 May 2026 20:48:56 +0800 Subject: [PATCH 3/6] feat: add skill branch review workflow --- docs/modules/02-skill-governance.md | 48 ++++-- .../layers/skill_governance/__init__.py | 10 ++ .../skill_governance/git_version_store.py | 34 ++++ .../skill_governance/skill_change_workflow.py | 117 ++++++++++++++ .../test_skill_governance_branch_review.py | 151 ++++++++++++++++++ 5 files changed, 346 insertions(+), 14 deletions(-) create mode 100644 skillos/skillos/layers/skill_governance/skill_change_workflow.py create mode 100644 skillos/tests/test_skill_governance_branch_review.py diff --git a/docs/modules/02-skill-governance.md b/docs/modules/02-skill-governance.md index 3735ce4..f52f43d 100644 --- a/docs/modules/02-skill-governance.md +++ b/docs/modules/02-skill-governance.md @@ -34,11 +34,11 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 `layers/skill_governance/git_version_store.py` -第一阶段新增的 Git 包装层，职责是安全调用 Git，而不是实现 Git： +Git 包装层，职责是安全调用 Git，而不是实现 Git： - 检查目标目录是否是 Git 仓库。 -- 获取当前分支。 -- 获取当前 HEAD commit。 +- 获取当前分支和 HEAD commit。 +- 创建、检查、切换本地分支。 - 获取某个 Skill 快照文件的 commit history。 - 获取两个 commit 之间的 Git diff。 - 在测试用临时仓库中创建 commit。 @@ -47,7 +47,7 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 `layers/skill_governance/skill_snapshot.py` -第二阶段新增 Skill 语义层能力： +Skill 语义层快照和 diff 能力： - 将 `Skill` 稳定序列化为 JSON 快照。 - 快照路径固定为 `skills//.json`。 @@ -63,9 +63,27 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 - 修改已有 schema property 的 `type`。 - 删除或清空已有 `implementation.prompt_template` / `implementation.code`。 -这些能力暂时不直接改 lifecycle API，也不自动写入真实项目仓库中的 Skill 快照。后续阶段会把它接入 branch / review / rollback 工作流。 +### 2.4 Skill Change Workflow -### 2.4 Lifecycle Management +`layers/skill_governance/skill_change_workflow.py` + +第三阶段新增的可审查变更工作流： + +- 根据新 Skill 版本生成固定分支名：`skill//-v`。 +- 在临时或指定 Git 仓库中创建变更分支。 +- 写入新版本 Skill snapshot。 +- 生成固定提交信息：`skill(): propose v`。 +- 返回 review bundle：分支名、base/head commit、snapshot 路径、commit message、diff、breaking 标记、建议 review 状态。 + +当前建议 review 状态： + +- `no_changes`：新旧快照没有变化，不创建分支和 commit。 +- `review_required`：存在非 breaking diff。 +- `breaking_review_required`：存在 breaking change。 + +这一层暂时不调用 LLM Reviewer，也不创建 GitHub PR。后续阶段再把 review bundle 接入 API、GitHub PR 或 E 前端展示。 + +### 2.5 Lifecycle Management `api/routes/lifecycle.py` @@ -83,9 +101,9 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 | `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史 diff | | `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个版本 | -前两阶段均不修改这些接口的请求和响应字段。 +前三阶段均不修改这些接口的请求和响应字段。 -### 2.5 Reviewer / Merger +### 2.6 Reviewer / Merger `layers/skill_governance/reviewer.py` @@ -123,14 +141,15 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 ### 阶段三：Branch / Review 工作流封装 -目标：把 Skill 修改映射为 Git branch 和 review 流程。 +目标：把 Skill 修改映射为 Git branch、snapshot commit 和 review bundle。 -重点： +已完成： -- Skill 修改分支。 -- commit message 规范。 -- review 状态与 lifecycle 状态对应。 -- 后续可接 GitHub PR，但不自造 PR 系统。 +- 新增本地分支检查、创建、切换封装。 +- 新增 `SkillChangeReviewBundle`。 +- 新增 `propose_skill_change()`。 +- 支持 `no_changes` / `review_required` / `breaking_review_required` 三种规则型预审状态。 +- 新增临时 Git 仓库分支和 review bundle 测试。 ### 阶段四：Rollback / Tag / Release @@ -161,3 +180,4 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 - 不改现有 lifecycle API 字段。 - 不碰 E 的 `frontend-dev` PR。 - 不碰 D 的 `agents-dev` PR。 +- 不自动创建 GitHub PR。 diff --git a/skillos/skillos/layers/skill_governance/__init__.py b/skillos/skillos/layers/skill_governance/__init__.py index 58e184d..994ae25 100644 --- a/skillos/skillos/layers/skill_governance/__init__.py +++ b/skillos/skillos/layers/skill_governance/__init__.py @@ -3,6 +3,12 @@ from .git_version_store import GitCommit, GitVersionStore, GitVersionStoreError from .merger import MergeResult, SkillMerger, SplitResult from .reviewer import ReviewResult, ReviewStatus, SkillReviewer +from .skill_change_workflow import ( + SkillChangeReviewBundle, + propose_skill_change, + skill_change_branch_name, + skill_change_commit_message, +) from .skill_snapshot import ( SkillSnapshotDiff, commit_skill_snapshot, @@ -20,6 +26,10 @@ "GitVersionStore", "GitVersionStoreError", "GitCommit", + "SkillChangeReviewBundle", + "skill_change_branch_name", + "skill_change_commit_message", + "propose_skill_change", "SkillSnapshotDiff", "skill_snapshot_path", "skill_to_snapshot", diff --git a/skillos/skillos/layers/skill_governance/git_version_store.py b/skillos/skillos/layers/skill_governance/git_version_store.py index e0ccd04..5018756 100644 --- a/skillos/skillos/layers/skill_governance/git_version_store.py +++ b/skillos/skillos/layers/skill_governance/git_version_store.py @@ -47,6 +47,30 @@ def current_branch(self) -> str: self._require_repo() return self._run(["rev-parse", "--abbrev-ref", "HEAD"]).strip() + def branch_exists(self, branch_name: str) -> bool: + """Return True when a local branch exists.""" + self._require_repo() + branch = self._normalize_branch_name(branch_name) + try: + self._run(["show-ref", "--verify", "--quiet", f"refs/heads/{branch}"]) + except GitVersionStoreError: + return False + return True + + def create_branch(self, branch_name: str, start_point: str = "HEAD") -> None: + """Create a local branch at start_point.""" + self._require_repo() + branch = self._normalize_branch_name(branch_name) + if self.branch_exists(branch): + raise GitVersionStoreError(f"Git branch already exists: {branch}") + self._run(["branch", branch, start_point]) + + def checkout(self, branch_name: str) -> None: + """Checkout an existing local branch.""" + self._require_repo() + branch = self._normalize_branch_name(branch_name) + self._run(["checkout", branch]) + def head_commit(self) -> str: """Return the current HEAD commit hash.""" self._require_repo() @@ -155,6 +179,16 @@ def _normalize_repo_path(path: str | Path) -> str: raise ValueError("Git path must be repo-relative and stay inside the repository.") return "/".join(parts) + @staticmethod + def _normalize_branch_name(branch_name: str) -> str: + branch = branch_name.strip().replace("\\", "/") + if not branch: + raise ValueError("Git branch name cannot be empty.") + parts = [part for part in branch.split("/") if part] + if ".." in branch or branch.startswith("/") or branch.endswith("/") or any(part == "." for part in parts): + raise ValueError(f"Invalid Git branch name: {branch_name!r}") + return "/".join(parts) + @staticmethod def _parse_history(output: str, marker: str) -> List[GitCommit]: commits: List[GitCommit] = [] diff --git a/skillos/skillos/layers/skill_governance/skill_change_workflow.py b/skillos/skillos/layers/skill_governance/skill_change_workflow.py new file mode 100644 index 0000000..4f50a3f --- /dev/null +++ b/skillos/skillos/layers/skill_governance/skill_change_workflow.py @@ -0,0 +1,117 @@ +"""Skill change workflow backed by Git branches and snapshot commits.""" + +from __future__ import annotations + +import re +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Dict, List, Optional + +from ...models.skill_model import Skill +from .git_version_store import GitVersionStore, GitVersionStoreError +from .skill_snapshot import ( + SkillSnapshotDiff, + diff_skill_snapshots, + has_breaking_changes, + skill_snapshot_path, + skill_to_snapshot, + write_skill_snapshot, +) + + +@dataclass(frozen=True) +class SkillChangeReviewBundle: + """Review bundle for a proposed Skill snapshot change.""" + + branch_name: str + base_commit: str + head_commit: str + snapshot_path: str + commit_message: str + diffs: List[SkillSnapshotDiff] + has_breaking_changes: bool + suggested_review_status: str + + def to_dict(self) -> Dict[str, Any]: + return { + "branch_name": self.branch_name, + "base_commit": self.base_commit, + "head_commit": self.head_commit, + "snapshot_path": self.snapshot_path, + "commit_message": self.commit_message, + "diffs": [diff.to_dict() for diff in self.diffs], + "has_breaking_changes": self.has_breaking_changes, + "suggested_review_status": self.suggested_review_status, + } + + +def skill_change_branch_name(skill: Skill) -> str: + """Build a deterministic branch name for a proposed Skill version.""" + safe_name = _slug(skill.name) + skill_id_prefix = _slug(skill.skill_id[:8]) + safe_version = _slug(skill.version) + return f"skill/{safe_name}/{skill_id_prefix}-v{safe_version}" + + +def skill_change_commit_message(skill: Skill) -> str: + return f"skill({skill.name}): propose v{skill.version}" + + +def propose_skill_change( + repo_path: str | Path, + old_skill: Skill, + new_skill: Skill, + store: Optional[GitVersionStore] = None, +) -> SkillChangeReviewBundle: + """Create a Git branch and snapshot commit for a proposed Skill change.""" + version_store = store or GitVersionStore(repo_path) + base_branch = version_store.current_branch() + base_commit = version_store.head_commit() + branch_name = skill_change_branch_name(new_skill) + snapshot_path = skill_snapshot_path(new_skill) + commit_message = skill_change_commit_message(new_skill) + diffs = diff_skill_snapshots(skill_to_snapshot(old_skill), skill_to_snapshot(new_skill)) + breaking = has_breaking_changes(diffs) + + if not diffs: + return SkillChangeReviewBundle( + branch_name=branch_name, + base_commit=base_commit, + head_commit=base_commit, + snapshot_path=snapshot_path, + commit_message="", + diffs=[], + has_breaking_changes=False, + suggested_review_status="no_changes", + ) + + if version_store.branch_exists(branch_name): + raise GitVersionStoreError(f"Git branch already exists: {branch_name}") + + try: + version_store.create_branch(branch_name, base_commit) + version_store.checkout(branch_name) + written_path = write_skill_snapshot(repo_path, new_skill) + head_commit = version_store.commit_paths([written_path], commit_message) + except Exception: + version_store.checkout(base_branch) + raise + + return SkillChangeReviewBundle( + branch_name=branch_name, + base_commit=base_commit, + head_commit=head_commit, + snapshot_path=snapshot_path, + commit_message=commit_message, + diffs=diffs, + has_breaking_changes=breaking, + suggested_review_status=( + "breaking_review_required" if breaking else "review_required" + ), + ) + + +def _slug(value: str) -> str: + slug = re.sub(r"[^A-Za-z0-9._-]+", "-", value.strip()) + slug = re.sub(r"-+", "-", slug).strip("-._") + return slug or "unknown" diff --git a/skillos/tests/test_skill_governance_branch_review.py b/skillos/tests/test_skill_governance_branch_review.py new file mode 100644 index 0000000..1516a76 --- /dev/null +++ b/skillos/tests/test_skill_governance_branch_review.py @@ -0,0 +1,151 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from skillos.layers.skill_governance import ( + GitVersionStore, + GitVersionStoreError, + propose_skill_change, + skill_change_branch_name, + skill_snapshot_path, +) +from skillos.models import Skill, SkillImplementation, SkillInterface, SkillState, SkillType + + +def _run_git(repo: Path, args: list[str]) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +@pytest.fixture +def git_repo(tmp_path: Path) -> Path: + _run_git(tmp_path, ["init"]) + baseline = tmp_path / ".gitkeep" + baseline.write_text("", encoding="utf-8") + store = GitVersionStore(tmp_path) + store.commit_paths([".gitkeep"], "initial baseline") + return tmp_path + + +def _skill(version: str = "1.0.0", description: str = "Search wiki entries.") -> Skill: + return Skill( + skill_id="skill-1234567890", + name="search_wiki", + version=version, + description=description, + skill_type=SkillType.FUNCTIONAL, + domain="wiki", + state=SkillState.RELEASED, + tags=["wiki", "search"], + interface=SkillInterface( + input_schema={ + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + output_schema={ + "type": "object", + "properties": {"results": {"type": "array"}}, + }, + ), + implementation=SkillImplementation(prompt_template="Search for {query}"), + ) + + +def test_branch_name_is_deterministic_and_sanitized() -> None: + skill = _skill(version="1.0.1") + skill.skill_id = "Skill 1234/5678" + skill.name = "search_wiki" + + assert skill_change_branch_name(skill) == "skill/search_wiki/Skill-12-v1.0.1" + + +def test_workflow_creates_review_branch_and_snapshot_commit(git_repo: Path) -> None: + old_skill = _skill() + new_skill = _skill(version="1.0.1", description="Search wiki entries with filters.") + store = GitVersionStore(git_repo) + base_branch = store.current_branch() + + bundle = propose_skill_change(git_repo, old_skill, new_skill, store) + + assert bundle.branch_name == "skill/search_wiki/skill-12-v1.0.1" + assert bundle.snapshot_path == "skills/skill-1234567890/1.0.1.json" + assert bundle.commit_message == "skill(search_wiki): propose v1.0.1" + assert bundle.suggested_review_status == "review_required" + assert not bundle.has_breaking_changes + assert store.current_branch() == bundle.branch_name + + history = store.commit_history(skill_snapshot_path(new_skill)) + assert history[0].subject == "skill(search_wiki): propose v1.0.1" + assert (git_repo / bundle.snapshot_path).exists() + store.checkout(base_branch) + + +def test_breaking_change_requires_breaking_review(git_repo: Path) -> None: + old_skill = _skill() + new_skill = _skill(version="2.0.0") + new_skill.interface.input_schema = { + "type": "object", + "properties": {"query": {"type": "array"}}, + "required": ["query"], + } + + bundle = propose_skill_change(git_repo, old_skill, new_skill) + + assert bundle.suggested_review_status == "breaking_review_required" + assert bundle.has_breaking_changes + assert any(diff.is_breaking for diff in bundle.diffs) + + +def test_no_changes_does_not_create_branch_or_commit(git_repo: Path) -> None: + old_skill = _skill() + new_skill = _skill() + store = GitVersionStore(git_repo) + base_branch = store.current_branch() + base_commit = store.head_commit() + + bundle = propose_skill_change(git_repo, old_skill, new_skill, store) + + assert bundle.suggested_review_status == "no_changes" + assert bundle.head_commit == base_commit + assert bundle.commit_message == "" + assert store.current_branch() == base_branch + assert not store.branch_exists(bundle.branch_name) + + +def test_existing_branch_fails_clearly(git_repo: Path) -> None: + old_skill = _skill() + new_skill = _skill(version="1.0.1", description="Updated description.") + store = GitVersionStore(git_repo) + store.create_branch(skill_change_branch_name(new_skill)) + + with pytest.raises(GitVersionStoreError, match="already exists"): + propose_skill_change(git_repo, old_skill, new_skill, store) + + +def test_workflow_failure_restores_base_branch(git_repo: Path) -> None: + old_skill = _skill() + new_skill = _skill(version="1.0.1", description="Updated description.") + store = GitVersionStore(git_repo) + base_branch = store.current_branch() + + class FailingCommitStore(GitVersionStore): + def commit_paths(self, paths, message, author_name="SkillOS", author_email="skillos@example.local"): + raise GitVersionStoreError("simulated commit failure") + + failing_store = FailingCommitStore(git_repo) + + with pytest.raises(GitVersionStoreError, match="simulated commit failure"): + propose_skill_change(git_repo, old_skill, new_skill, failing_store) + + assert store.current_branch() == base_branch From 05df302f9e9d23452930200f600a1edd46f3296a Mon Sep 17 00:00:00 2001 From: DYD Date: Sun, 3 May 2026 21:32:24 +0800 Subject: [PATCH 4/6] feat: add skill release rollback --- docs/modules/02-skill-governance.md | 87 +++++++--- .../layers/skill_governance/__init__.py | 14 ++ .../skill_governance/git_version_store.py | 37 +++++ .../layers/skill_governance/skill_release.py | 135 ++++++++++++++++ .../test_skill_governance_release_rollback.py | 148 ++++++++++++++++++ 5 files changed, 396 insertions(+), 25 deletions(-) create mode 100644 skillos/skillos/layers/skill_governance/skill_release.py create mode 100644 skillos/tests/test_skill_governance_release_rollback.py diff --git a/docs/modules/02-skill-governance.md b/docs/modules/02-skill-governance.md index f52f43d..ca8e850 100644 --- a/docs/modules/02-skill-governance.md +++ b/docs/modules/02-skill-governance.md @@ -6,14 +6,19 @@ ## 职责概览 -Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包括： +Skill Governance Layer 负责 Skill 的生命周期治理和版本治理。当前路线不是重新实现一套 Git-like 系统，而是把真实 Git 作为底层版本事实来源，再由 SkillOS 在上面增加 Skill 语义。 -- Skill 生命周期状态流转：S0-S7。 -- Skill 审核、发布、废弃、修复后的治理记录。 -- Skill 版本历史、diff、breaking change 判断。 -- 基于 Git 的 branch / commit / history / diff / rollback / PR 工作流封装。 +换句话说： -当前方向不是重新实现一个 Git-like 系统，而是把 Git 作为底层版本事实来源。SkillOS 只在 Git 之上增加 Skill 语义，例如“某次 commit 对应哪个 Skill 版本”“某个 diff 是否是 breaking change”“某次修复是否需要 review”。 +- Git 负责 branch、commit、diff、history、tag 等成熟版本能力。 +- SkillOS 负责解释“某个 commit 对应哪个 Skill 版本”“某个 diff 是否是 breaking change”“某次修改是否需要 review”“某次恢复来自哪个历史版本”。 + +当前 B 任务已经形成四层基础能力： + +- Git-backed Version Adapter +- Skill Snapshot / Domain Diff / Breaking Change 检测 +- Branch / Review 工作流封装 +- Release Tag / Restore Commit 回滚 --- @@ -23,25 +28,29 @@ Skill Governance Layer 负责 Skill 的生命周期治理与版本治理，包 `layers/skill_governance/version_control.py` -保留现有语义化版本控制能力： +保留原有语义化版本控制能力： - 记录 `ChangeRecord`。 - 计算两个 Skill 对象之间的字段级 diff。 - 根据 diff 建议 `major` / `minor` / `patch`。 -- 创建新版本时把 Skill 回到 Draft 状态，等待后续审核。 +- 创建新版本时让 Skill 回到 Draft 状态，等待后续审核。 ### 2.2 Git Version Store `layers/skill_governance/git_version_store.py` -Git 包装层，职责是安全调用 Git，而不是实现 Git： +Git 包装层只负责安全调用 Git，不重新实现 Git： - 检查目标目录是否是 Git 仓库。 - 获取当前分支和 HEAD commit。 - 创建、检查、切换本地分支。 -- 获取某个 Skill 快照文件的 commit history。 +- 获取指定文件的 commit history。 - 获取两个 commit 之间的 Git diff。 - 在测试用临时仓库中创建 commit。 +- 检查和创建本地 lightweight tag。 +- 从 commit、branch 或 tag 读取指定文件内容。 + +所有 Git 命令都通过 `subprocess` 调用，并带有超时、错误捕获和清晰异常信息。 ### 2.3 Skill Snapshot / Domain Diff @@ -73,7 +82,7 @@ Skill 语义层快照和 diff 能力： - 在临时或指定 Git 仓库中创建变更分支。 - 写入新版本 Skill snapshot。 - 生成固定提交信息：`skill(): propose v`。 -- 返回 review bundle：分支名、base/head commit、snapshot 路径、commit message、diff、breaking 标记、建议 review 状态。 +- 返回 review bundle，包括分支名、base/head commit、snapshot 路径、commit message、diff、breaking 标记和建议 review 状态。 当前建议 review 状态： @@ -81,9 +90,30 @@ Skill 语义层快照和 diff 能力： - `review_required`：存在非 breaking diff。 - `breaking_review_required`：存在 breaking change。 -这一层暂时不调用 LLM Reviewer，也不创建 GitHub PR。后续阶段再把 review bundle 接入 API、GitHub PR 或 E 前端展示。 +这一层暂时不调用 LLM Reviewer，也不创建 GitHub PR。后续阶段可以把 review bundle 接入 API、GitHub PR 或 E 前端展示。 + +### 2.5 Skill Release / Rollback + +`layers/skill_governance/skill_release.py` + +第四阶段新增 release tag 和 restore commit 回滚能力： + +- Skill release tag 规则：`skill///v`。 +- `release_skill_snapshot()` 会确认目标 ref 下存在对应 Skill snapshot，然后创建本地 lightweight tag。 +- `read_skill_snapshot_at_ref()` 可以从 commit、branch 或 tag 读取历史 snapshot JSON。 +- `restore_skill_snapshot()` 会从历史 ref 读取 snapshot，把内容写回当前工作树，并创建一个新的 restore commit。 -### 2.5 Lifecycle Management +回滚策略非常关键：这里的回滚不是 `git reset`，也不是破坏性覆盖历史，而是新增恢复提交。这样 Git 历史会完整保留，组长 review 时也能看见“从哪个 tag/commit 恢复到了当前内容”。 + +当前 restore commit 信息固定为： + +```text +skill(): restore from +``` + +第四阶段只做本地治理能力，不做远端 tag push，不创建 GitHub Release，也不新增 REST API 或前端页面。 + +### 2.6 Lifecycle Management `api/routes/lifecycle.py` @@ -101,15 +131,15 @@ Skill 语义层快照和 diff 能力： | `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史 diff | | `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个版本 | -前三阶段均不修改这些接口的请求和响应字段。 +B 前四阶段均不修改这些接口的请求和响应字段。 -### 2.6 Reviewer / Merger +### 2.7 Reviewer / Merger `layers/skill_governance/reviewer.py` `layers/skill_governance/merger.py` -现有 Reviewer 负责 Skill 质量审核，Merger 负责相似 Skill 合并和大 Skill 拆分。B 后续阶段会让这些治理动作也产生 Git-backed 版本记录。 +现有 Reviewer 负责 Skill 质量审核，Merger 负责相似 Skill 合并和大 Skill 拆分。后续可以让这些治理动作也写入 Git-backed snapshot 和 review workflow，形成可追踪版本记录。 --- @@ -124,7 +154,7 @@ Skill 语义层快照和 diff 能力： - 新增 `GitVersionStore`。 - 新增临时 Git 仓库测试。 - 保持 REST 接口不变。 -- 更新本模块文档，明确“Git 外壳”路线。 +- 明确“Git 外壳”路线。 ### 阶段二：Skill 快照与领域级 Diff @@ -151,25 +181,30 @@ Skill 语义层快照和 diff 能力： - 支持 `no_changes` / `review_required` / `breaking_review_required` 三种规则型预审状态。 - 新增临时 Git 仓库分支和 review bundle 测试。 -### 阶段四：Rollback / Tag / Release +### 阶段四：Release Tag 与 Restore Commit 回滚 -目标：支持按 commit 或版本 tag 回滚 Skill。 +目标：支持给 Skill snapshot commit 打 release tag，并能从历史 tag/commit 读取 snapshot 后创建恢复提交。 -重点： +已完成： -- version tag 规则。 -- rollback API 设计。 -- rollback 后的 Skill 状态与审计记录。 +- 新增 tag 检查和创建能力。 +- 新增 ref 下文件读取能力。 +- 新增 release record 和 rollback record。 +- 新增 `release_skill_snapshot()`。 +- 新增 `read_skill_snapshot_at_ref()`。 +- 新增 `restore_skill_snapshot()`。 +- 新增 release / rollback 临时 Git 仓库测试。 ### 阶段五：联调与交付 -目标：与 E 前端和 D 自管理 Agent 联调。 +目标：与 E 前端和 D 自管理 Agent 联调，并准备中文 PR 与组长审核材料。 重点： - E 展示 Skill 版本历史和 diff。 - D 的 repair / merge / split 产生可追踪版本记录。 -- 中文 PR 说明、交付文档、组长 review。 +- 根据团队安排决定是否新增 REST 接口或前端页面。 +- 准备中文 PR 说明、交付文档和 review 重点。 --- @@ -181,3 +216,5 @@ Skill 语义层快照和 diff 能力： - 不碰 E 的 `frontend-dev` PR。 - 不碰 D 的 `agents-dev` PR。 - 不自动创建 GitHub PR。 +- 不 push release tag。 +- 不使用 `git reset --hard`、`git checkout -- `、`git clean` 等破坏性回滚操作。 diff --git a/skillos/skillos/layers/skill_governance/__init__.py b/skillos/skillos/layers/skill_governance/__init__.py index 994ae25..42bd376 100644 --- a/skillos/skillos/layers/skill_governance/__init__.py +++ b/skillos/skillos/layers/skill_governance/__init__.py @@ -9,6 +9,14 @@ skill_change_branch_name, skill_change_commit_message, ) +from .skill_release import ( + SkillReleaseRecord, + SkillRollbackRecord, + read_skill_snapshot_at_ref, + release_skill_snapshot, + restore_skill_snapshot, + skill_release_tag_name, +) from .skill_snapshot import ( SkillSnapshotDiff, commit_skill_snapshot, @@ -30,6 +38,12 @@ "skill_change_branch_name", "skill_change_commit_message", "propose_skill_change", + "SkillReleaseRecord", + "SkillRollbackRecord", + "skill_release_tag_name", + "release_skill_snapshot", + "read_skill_snapshot_at_ref", + "restore_skill_snapshot", "SkillSnapshotDiff", "skill_snapshot_path", "skill_to_snapshot", diff --git a/skillos/skillos/layers/skill_governance/git_version_store.py b/skillos/skillos/layers/skill_governance/git_version_store.py index 5018756..d560921 100644 --- a/skillos/skillos/layers/skill_governance/git_version_store.py +++ b/skillos/skillos/layers/skill_governance/git_version_store.py @@ -71,6 +71,33 @@ def checkout(self, branch_name: str) -> None: branch = self._normalize_branch_name(branch_name) self._run(["checkout", branch]) + def tag_exists(self, tag_name: str) -> bool: + """Return True when a local tag exists.""" + self._require_repo() + tag = self._normalize_tag_name(tag_name) + try: + self._run(["show-ref", "--verify", "--quiet", f"refs/tags/{tag}"]) + except GitVersionStoreError: + return False + return True + + def create_tag(self, tag_name: str, ref: str = "HEAD") -> None: + """Create a lightweight local tag for a ref.""" + self._require_repo() + tag = self._normalize_tag_name(tag_name) + if self.tag_exists(tag): + raise GitVersionStoreError(f"Git tag already exists: {tag}") + self._run(["tag", tag, ref]) + + def read_file_at_ref(self, ref: str, path: str | Path) -> str: + """Read a repo-relative file from a commit, branch, or tag.""" + self._require_repo() + git_path = self._normalize_repo_path(path) + clean_ref = ref.strip() + if not clean_ref: + raise ValueError("Git ref cannot be empty.") + return self._run(["show", f"{clean_ref}:{git_path}"]) + def head_commit(self) -> str: """Return the current HEAD commit hash.""" self._require_repo() @@ -189,6 +216,16 @@ def _normalize_branch_name(branch_name: str) -> str: raise ValueError(f"Invalid Git branch name: {branch_name!r}") return "/".join(parts) + @staticmethod + def _normalize_tag_name(tag_name: str) -> str: + tag = tag_name.strip().replace("\\", "/") + if not tag: + raise ValueError("Git tag name cannot be empty.") + parts = [part for part in tag.split("/") if part] + if ".." in tag or tag.startswith("/") or tag.endswith("/") or any(part == "." for part in parts): + raise ValueError(f"Invalid Git tag name: {tag_name!r}") + return "/".join(parts) + @staticmethod def _parse_history(output: str, marker: str) -> List[GitCommit]: commits: List[GitCommit] = [] diff --git a/skillos/skillos/layers/skill_governance/skill_release.py b/skillos/skillos/layers/skill_governance/skill_release.py new file mode 100644 index 0000000..bca34c6 --- /dev/null +++ b/skillos/skillos/layers/skill_governance/skill_release.py @@ -0,0 +1,135 @@ +"""Skill release tags and restore-commit rollback utilities.""" + +from __future__ import annotations + +import json +import re +from dataclasses import dataclass +from pathlib import Path +from typing import Any, Dict, Optional + +from ...models.skill_model import Skill +from .git_version_store import GitVersionStore +from .skill_snapshot import skill_snapshot_path + + +@dataclass(frozen=True) +class SkillReleaseRecord: + tag_name: str + commit: str + snapshot_path: str + skill_id: str + skill_name: str + version: str + + def to_dict(self) -> Dict[str, Any]: + return { + "tag_name": self.tag_name, + "commit": self.commit, + "snapshot_path": self.snapshot_path, + "skill_id": self.skill_id, + "skill_name": self.skill_name, + "version": self.version, + } + + +@dataclass(frozen=True) +class SkillRollbackRecord: + source_ref: str + restore_commit: str + restored_snapshot_path: str + commit_message: str + skill_id: str + skill_name: str + version: str + + def to_dict(self) -> Dict[str, Any]: + return { + "source_ref": self.source_ref, + "restore_commit": self.restore_commit, + "restored_snapshot_path": self.restored_snapshot_path, + "commit_message": self.commit_message, + "skill_id": self.skill_id, + "skill_name": self.skill_name, + "version": self.version, + } + + +def skill_release_tag_name(skill: Skill) -> str: + safe_name = _slug(skill.name) + skill_id_prefix = _slug(skill.skill_id[:8]) + safe_version = _slug(skill.version) + return f"skill/{safe_name}/{skill_id_prefix}/v{safe_version}" + + +def release_skill_snapshot( + repo_path: str | Path, + skill: Skill, + ref: str = "HEAD", + store: Optional[GitVersionStore] = None, +) -> SkillReleaseRecord: + version_store = store or GitVersionStore(repo_path) + tag_name = skill_release_tag_name(skill) + snapshot_path = skill_snapshot_path(skill) + + if version_store.tag_exists(tag_name): + raise ValueError(f"Skill release tag already exists: {tag_name}") + + version_store.read_file_at_ref(ref, snapshot_path) + version_store.create_tag(tag_name, ref) + + return SkillReleaseRecord( + tag_name=tag_name, + commit=version_store.head_commit() if ref == "HEAD" else ref, + snapshot_path=snapshot_path, + skill_id=skill.skill_id, + skill_name=skill.name, + version=skill.version, + ) + + +def read_skill_snapshot_at_ref( + repo_path: str | Path, + ref: str, + snapshot_path: str, + store: Optional[GitVersionStore] = None, +) -> Dict[str, Any]: + version_store = store or GitVersionStore(repo_path) + return json.loads(version_store.read_file_at_ref(ref, snapshot_path)) + + +def restore_skill_snapshot( + repo_path: str | Path, + current_skill: Skill, + source_ref: str, + store: Optional[GitVersionStore] = None, +) -> SkillRollbackRecord: + version_store = store or GitVersionStore(repo_path) + snapshot_path = skill_snapshot_path(current_skill) + snapshot = read_skill_snapshot_at_ref(repo_path, source_ref, snapshot_path, version_store) + + target = Path(repo_path) / snapshot_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text( + json.dumps(snapshot, ensure_ascii=False, sort_keys=True, indent=2) + "\n", + encoding="utf-8", + ) + + commit_message = f"skill({current_skill.name}): restore from {source_ref}" + restore_commit = version_store.commit_paths([snapshot_path], commit_message) + + return SkillRollbackRecord( + source_ref=source_ref, + restore_commit=restore_commit, + restored_snapshot_path=snapshot_path, + commit_message=commit_message, + skill_id=current_skill.skill_id, + skill_name=current_skill.name, + version=current_skill.version, + ) + + +def _slug(value: str) -> str: + slug = re.sub(r"[^A-Za-z0-9._-]+", "-", value.strip()) + slug = re.sub(r"-+", "-", slug).strip("-._") + return slug or "unknown" diff --git a/skillos/tests/test_skill_governance_release_rollback.py b/skillos/tests/test_skill_governance_release_rollback.py new file mode 100644 index 0000000..b74d7f0 --- /dev/null +++ b/skillos/tests/test_skill_governance_release_rollback.py @@ -0,0 +1,148 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + +from skillos.layers.skill_governance import ( + GitVersionStore, + GitVersionStoreError, + read_skill_snapshot_at_ref, + release_skill_snapshot, + restore_skill_snapshot, + skill_release_tag_name, + skill_snapshot_path, + write_skill_snapshot, +) +from skillos.models import Skill, SkillImplementation, SkillInterface, SkillState, SkillType + + +def _run_git(repo: Path, args: list[str]) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +@pytest.fixture +def git_repo(tmp_path: Path) -> Path: + _run_git(tmp_path, ["init"]) + baseline = tmp_path / ".gitkeep" + baseline.write_text("", encoding="utf-8") + GitVersionStore(tmp_path).commit_paths([".gitkeep"], "initial baseline") + return tmp_path + + +def _skill(version: str = "1.0.0", description: str = "Search wiki entries.") -> Skill: + return Skill( + skill_id="skill-1234567890", + name="search_wiki", + version=version, + description=description, + skill_type=SkillType.FUNCTIONAL, + domain="wiki", + state=SkillState.RELEASED, + tags=["wiki", "search"], + interface=SkillInterface( + input_schema={ + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + output_schema={ + "type": "object", + "properties": {"results": {"type": "array"}}, + }, + ), + implementation=SkillImplementation(prompt_template="Search for {query}"), + ) + + +def _commit_snapshot(repo: Path, skill: Skill, message: str) -> str: + snapshot_path = write_skill_snapshot(repo, skill) + return GitVersionStore(repo).commit_paths([snapshot_path], message) + + +def test_release_tag_name_is_deterministic_and_sanitized() -> None: + skill = _skill() + skill.skill_id = "Skill 1234/5678" + + assert skill_release_tag_name(skill) == "skill/search_wiki/Skill-12/v1.0.0" + + +def test_release_skill_snapshot_creates_tag(git_repo: Path) -> None: + skill = _skill() + commit_hash = _commit_snapshot(git_repo, skill, "skill(search_wiki): snapshot v1.0.0") + store = GitVersionStore(git_repo) + + record = release_skill_snapshot(git_repo, skill, commit_hash, store) + + assert record.tag_name == "skill/search_wiki/skill-12/v1.0.0" + assert record.commit == commit_hash + assert record.snapshot_path == "skills/skill-1234567890/1.0.0.json" + assert store.tag_exists(record.tag_name) + + +def test_duplicate_release_tag_fails_clearly(git_repo: Path) -> None: + skill = _skill() + commit_hash = _commit_snapshot(git_repo, skill, "skill(search_wiki): snapshot v1.0.0") + + release_skill_snapshot(git_repo, skill, commit_hash) + + with pytest.raises(ValueError, match="already exists"): + release_skill_snapshot(git_repo, skill, commit_hash) + + +def test_read_file_at_ref_reads_snapshot_from_tag(git_repo: Path) -> None: + skill = _skill(description="Released description.") + commit_hash = _commit_snapshot(git_repo, skill, "skill(search_wiki): snapshot v1.0.0") + tag = release_skill_snapshot(git_repo, skill, commit_hash).tag_name + + snapshot = read_skill_snapshot_at_ref(git_repo, tag, skill_snapshot_path(skill)) + + assert snapshot["name"] == "search_wiki" + assert snapshot["description"] == "Released description." + + +def test_restore_skill_snapshot_creates_restore_commit(git_repo: Path) -> None: + old_skill = _skill(description="Original description.") + old_commit = _commit_snapshot(git_repo, old_skill, "skill(search_wiki): snapshot v1.0.0") + old_tag = release_skill_snapshot(git_repo, old_skill, old_commit).tag_name + + current_skill = _skill(description="Changed description.") + _commit_snapshot(git_repo, current_skill, "skill(search_wiki): snapshot changed") + + record = restore_skill_snapshot(git_repo, current_skill, old_tag) + store = GitVersionStore(git_repo) + restored_snapshot = read_skill_snapshot_at_ref(git_repo, "HEAD", skill_snapshot_path(current_skill)) + history = store.commit_history(skill_snapshot_path(current_skill)) + + assert record.source_ref == old_tag + assert record.commit_message == f"skill(search_wiki): restore from {old_tag}" + assert restored_snapshot["description"] == "Original description." + assert history[0].subject == record.commit_message + assert any(entry.commit_hash == old_commit for entry in history) + + +def test_restore_missing_ref_fails_clearly(git_repo: Path) -> None: + skill = _skill() + + with pytest.raises(GitVersionStoreError, match="Git command failed"): + restore_skill_snapshot(git_repo, skill, "missing-ref") + + +def test_restore_missing_snapshot_fails_clearly(git_repo: Path) -> None: + other_file = git_repo / "notes.txt" + other_file.write_text("no skill snapshot here\n", encoding="utf-8") + store = GitVersionStore(git_repo) + commit_hash = store.commit_paths(["notes.txt"], "add notes") + store.create_tag("notes-tag", commit_hash) + + with pytest.raises(GitVersionStoreError, match="Git command failed"): + restore_skill_snapshot(git_repo, _skill(), "notes-tag") From 95bb51017f51a5bb18937fe1a2bc9d8fc960afdf Mon Sep 17 00:00:00 2001 From: DYD Date: Sun, 3 May 2026 21:52:24 +0800 Subject: [PATCH 5/6] feat: expose skill governance api --- docs/modules/02-skill-governance.md | 77 +++--- skillos/skillos/api/routes/lifecycle.py | 149 ++++++++++++ skillos/skillos/api/schemas.py | 38 +++ .../test_skill_governance_lifecycle_api.py | 224 ++++++++++++++++++ 4 files changed, 460 insertions(+), 28 deletions(-) create mode 100644 skillos/tests/test_skill_governance_lifecycle_api.py diff --git a/docs/modules/02-skill-governance.md b/docs/modules/02-skill-governance.md index ca8e850..33cb4a3 100644 --- a/docs/modules/02-skill-governance.md +++ b/docs/modules/02-skill-governance.md @@ -13,12 +13,13 @@ Skill Governance Layer 负责 Skill 的生命周期治理和版本治理。当 - Git 负责 branch、commit、diff、history、tag 等成熟版本能力。 - SkillOS 负责解释“某个 commit 对应哪个 Skill 版本”“某个 diff 是否是 breaking change”“某次修改是否需要 review”“某次恢复来自哪个历史版本”。 -当前 B 任务已经形成四层基础能力： +当前 B 任务已经形成五层能力： - Git-backed Version Adapter - Skill Snapshot / Domain Diff / Breaking Change 检测 - Branch / Review 工作流封装 - Release Tag / Restore Commit 回滚 +- 最小 REST API 联调入口 --- @@ -76,7 +77,7 @@ Skill 语义层快照和 diff 能力： `layers/skill_governance/skill_change_workflow.py` -第三阶段新增的可审查变更工作流： +可审查变更工作流： - 根据新 Skill 版本生成固定分支名：`skill//-v`。 - 在临时或指定 Git 仓库中创建变更分支。 @@ -90,20 +91,20 @@ Skill 语义层快照和 diff 能力： - `review_required`：存在非 breaking diff。 - `breaking_review_required`：存在 breaking change。 -这一层暂时不调用 LLM Reviewer，也不创建 GitHub PR。后续阶段可以把 review bundle 接入 API、GitHub PR 或 E 前端展示。 +这一层暂时不调用 LLM Reviewer，也不创建 GitHub PR。后续可以把 review bundle 接入 API、GitHub PR 或 E 前端展示。 ### 2.5 Skill Release / Rollback `layers/skill_governance/skill_release.py` -第四阶段新增 release tag 和 restore commit 回滚能力： +Release tag 和 restore commit 回滚能力： - Skill release tag 规则：`skill///v`。 - `release_skill_snapshot()` 会确认目标 ref 下存在对应 Skill snapshot，然后创建本地 lightweight tag。 - `read_skill_snapshot_at_ref()` 可以从 commit、branch 或 tag 读取历史 snapshot JSON。 - `restore_skill_snapshot()` 会从历史 ref 读取 snapshot，把内容写回当前工作树，并创建一个新的 restore commit。 -回滚策略非常关键：这里的回滚不是 `git reset`，也不是破坏性覆盖历史，而是新增恢复提交。这样 Git 历史会完整保留，组长 review 时也能看见“从哪个 tag/commit 恢复到了当前内容”。 +回滚策略非常关键：这里的回滚不是 `git reset`，也不是破坏性覆盖历史，而是新增恢复提交。这样 Git 历史会完整保留，review 时也能看见“从哪个 tag/commit 恢复到了当前内容”。 当前 restore commit 信息固定为： @@ -111,13 +112,37 @@ Skill 语义层快照和 diff 能力： skill(): restore from ``` -第四阶段只做本地治理能力，不做远端 tag push，不创建 GitHub Release，也不新增 REST API 或前端页面。 +### 2.6 Lifecycle API Integration -### 2.6 Lifecycle Management +`api/routes/lifecycle.py` + +第五阶段新增最小 Git-backed 联调入口，全部放在现有 `/api/v1/lifecycle` 下： + +| 方法 | 路径 | 功能 | +| --- | --- | --- | +| `POST` | `/api/v1/lifecycle/{id}/snapshot` | 将当前 Skill 写成 snapshot 并创建 commit | +| `GET` | `/api/v1/lifecycle/{id}/snapshot/history` | 读取当前 Skill snapshot 的 Git commit history | +| `GET` | `/api/v1/lifecycle/{id}/snapshot/diff` | 返回 raw Git diff、结构化 diff 和 breaking 标记 | +| `POST` | `/api/v1/lifecycle/{id}/release-tag` | 为指定 ref 下的 Skill snapshot 创建本地 lightweight tag | +| `POST` | `/api/v1/lifecycle/{id}/rollback` | 从历史 ref 读取 snapshot 并创建 restore commit | + +Git 仓库路径策略： + +- 优先读取环境变量 `SKILLOS_GOVERNANCE_REPO`。 +- 未设置时默认使用当前项目 Git 仓库根目录。 +- 测试使用临时 Git 仓库，不污染真实项目仓库。 + +重要边界： + +- `rollback` 只恢复 Git snapshot 文件，不直接改 Wiki 中的 live Skill 对象。 +- 本阶段不 push tag，不创建 GitHub Release，不自动创建 GitHub PR。 +- 现有 lifecycle API 字段不变。 + +### 2.7 Existing Lifecycle Management `api/routes/lifecycle.py` -封装现有生命周期接口： +保留现有生命周期接口： | 方法 | 路径 | 功能 | | --- | --- | --- | @@ -131,9 +156,7 @@ skill(): restore from | `GET` | `/api/v1/lifecycle/{id}/diff` | 获取变更历史 diff | | `GET` | `/api/v1/lifecycle/{id}/diff/versions` | 比较两个版本 | -B 前四阶段均不修改这些接口的请求和响应字段。 - -### 2.7 Reviewer / Merger +### 2.8 Reviewer / Merger `layers/skill_governance/reviewer.py` @@ -143,12 +166,10 @@ B 前四阶段均不修改这些接口的请求和响应字段。 --- -## B 任务阶段规划 +## B 任务阶段状态 ### 阶段一：Git-backed Version Adapter 底座 -目标：先把 Git 调用能力放进 governance 层，保证后续不用自研 Git-like 版本系统。 - 已完成： - 新增 `GitVersionStore`。 @@ -158,8 +179,6 @@ B 前四阶段均不修改这些接口的请求和响应字段。 ### 阶段二：Skill 快照与领域级 Diff -目标：把 Skill 序列化为稳定 JSON 快照，并用 Git diff + Skill 字段级解释生成更可读的版本差异。 - 已完成： - 新增 Skill snapshot 稳定序列化。 @@ -171,8 +190,6 @@ B 前四阶段均不修改这些接口的请求和响应字段。 ### 阶段三：Branch / Review 工作流封装 -目标：把 Skill 修改映射为 Git branch、snapshot commit 和 review bundle。 - 已完成： - 新增本地分支检查、创建、切换封装。 @@ -183,8 +200,6 @@ B 前四阶段均不修改这些接口的请求和响应字段。 ### 阶段四：Release Tag 与 Restore Commit 回滚 -目标：支持给 Skill snapshot commit 打 release tag，并能从历史 tag/commit 读取 snapshot 后创建恢复提交。 - 已完成： - 新增 tag 检查和创建能力。 @@ -195,16 +210,22 @@ B 前四阶段均不修改这些接口的请求和响应字段。 - 新增 `restore_skill_snapshot()`。 - 新增 release / rollback 临时 Git 仓库测试。 -### 阶段五：联调与交付 +### 阶段五：最小 API 联调与 PR 交付 + +已完成： -目标：与 E 前端和 D 自管理 Agent 联调，并准备中文 PR 与组长审核材料。 +- 新增 Git-backed snapshot commit API。 +- 新增 snapshot history API。 +- 新增 snapshot diff API。 +- 新增 release-tag API。 +- 新增 restore commit rollback API。 +- 新增 lifecycle API 测试，使用临时 Git 仓库验证，不污染真实项目仓库。 -重点： +后续联调重点： -- E 展示 Skill 版本历史和 diff。 -- D 的 repair / merge / split 产生可追踪版本记录。 -- 根据团队安排决定是否新增 REST 接口或前端页面。 -- 准备中文 PR 说明、交付文档和 review 重点。 +- E 前端可以接入 `/versions` 页面展示 Git-backed history / diff。 +- D 的 repair / merge / split 后续可以调用 snapshot API 形成可追踪版本记录。 +- 由组长决定是否把这些新 API 同步到飞书锁定接口文档。 --- @@ -215,6 +236,6 @@ B 前四阶段均不修改这些接口的请求和响应字段。 - 不改现有 lifecycle API 字段。 - 不碰 E 的 `frontend-dev` PR。 - 不碰 D 的 `agents-dev` PR。 -- 不自动创建 GitHub PR。 +- 不自动合并 PR。 - 不 push release tag。 - 不使用 `git reset --hard`、`git checkout -- `、`git clean` 等破坏性回滚操作。 diff --git a/skillos/skillos/api/routes/lifecycle.py b/skillos/skillos/api/routes/lifecycle.py index da0cdfd..9af7757 100644 --- a/skillos/skillos/api/routes/lifecycle.py +++ b/skillos/skillos/api/routes/lifecycle.py @@ -2,17 +2,37 @@ from __future__ import annotations +import os +from pathlib import Path from typing import Any, Dict, List, Optional from fastapi import APIRouter, Depends, HTTPException +from ...layers.skill_governance import ( + GitVersionStore, + GitVersionStoreError, + diff_skill_snapshots, + has_breaking_changes, + read_skill_snapshot_at_ref, + release_skill_snapshot, + restore_skill_snapshot, + skill_snapshot_path, + skill_to_snapshot, + write_skill_snapshot, +) from ..deps import AppState, get_app_state from ..schemas import ( DeprecateRequest, NewVersionRequest, OKResponse, ReleaseRequest, + ReleaseTagRequest, + RollbackRequest, SkillSummary, + SnapshotCommitRequest, + SnapshotCommitResponse, + SnapshotDiffResponse, + SnapshotHistoryResponse, TransitionRequest, ) @@ -202,6 +222,121 @@ async def diff_two_versions( } +@router.post("/{skill_id}/snapshot", response_model=SnapshotCommitResponse) +async def commit_skill_snapshot_endpoint( + skill_id: str, + req: SnapshotCommitRequest, + app: AppState = Depends(get_app_state), +) -> SnapshotCommitResponse: + skill = await _get_skill_or_404(skill_id, app) + repo_path = _governance_repo_path() + store = GitVersionStore(repo_path) + snapshot_path = write_skill_snapshot(repo_path, skill) + message = req.message or f"skill({skill.name}): snapshot v{skill.version}" + try: + commit = store.commit_paths([snapshot_path], message, author_name=req.author) + except (GitVersionStoreError, ValueError) as e: + raise HTTPException(status_code=400, detail=str(e)) + return SnapshotCommitResponse( + skill_id=skill.skill_id, + skill_name=skill.name, + version=skill.version, + snapshot_path=snapshot_path, + commit=commit, + message=message, + ) + + +@router.get("/{skill_id}/snapshot/history", response_model=SnapshotHistoryResponse) +async def get_skill_snapshot_history( + skill_id: str, + max_count: int = 20, + app: AppState = Depends(get_app_state), +) -> SnapshotHistoryResponse: + skill = await _get_skill_or_404(skill_id, app) + repo_path = _governance_repo_path() + store = GitVersionStore(repo_path) + snapshot_path = skill_snapshot_path(skill) + try: + history = store.commit_history(snapshot_path, max_count=max_count) + except (GitVersionStoreError, ValueError) as e: + raise HTTPException(status_code=400, detail=str(e)) + return SnapshotHistoryResponse( + skill_id=skill.skill_id, + snapshot_path=snapshot_path, + history=[ + { + "commit_hash": item.commit_hash, + "author": item.author, + "authored_at": item.authored_at, + "subject": item.subject, + "changed_paths": list(item.changed_paths), + } + for item in history + ], + ) + + +@router.get("/{skill_id}/snapshot/diff", response_model=SnapshotDiffResponse) +async def get_skill_snapshot_diff( + skill_id: str, + from_ref: str, + to_ref: str = "HEAD", + app: AppState = Depends(get_app_state), +) -> SnapshotDiffResponse: + skill = await _get_skill_or_404(skill_id, app) + repo_path = _governance_repo_path() + store = GitVersionStore(repo_path) + snapshot_path = skill_snapshot_path(skill) + try: + raw_diff = store.diff_between(from_ref, to_ref, snapshot_path) + old_snapshot = read_skill_snapshot_at_ref(repo_path, from_ref, snapshot_path, store) + try: + new_snapshot = read_skill_snapshot_at_ref(repo_path, to_ref, snapshot_path, store) + except GitVersionStoreError: + new_snapshot = skill_to_snapshot(skill) + diffs = diff_skill_snapshots(old_snapshot, new_snapshot) + except (GitVersionStoreError, ValueError) as e: + raise HTTPException(status_code=400, detail=str(e)) + return SnapshotDiffResponse( + skill_id=skill.skill_id, + snapshot_path=snapshot_path, + from_ref=from_ref, + to_ref=to_ref, + raw_diff=raw_diff, + diffs=[item.to_dict() for item in diffs], + has_breaking_changes=has_breaking_changes(diffs), + ) + + +@router.post("/{skill_id}/release-tag", response_model=Dict[str, Any]) +async def create_skill_release_tag( + skill_id: str, + req: ReleaseTagRequest, + app: AppState = Depends(get_app_state), +) -> Dict[str, Any]: + skill = await _get_skill_or_404(skill_id, app) + repo_path = _governance_repo_path() + try: + return release_skill_snapshot(repo_path, skill, ref=req.ref).to_dict() + except (GitVersionStoreError, ValueError) as e: + raise HTTPException(status_code=400, detail=str(e)) + + +@router.post("/{skill_id}/rollback", response_model=Dict[str, Any]) +async def rollback_skill_snapshot( + skill_id: str, + req: RollbackRequest, + app: AppState = Depends(get_app_state), +) -> Dict[str, Any]: + skill = await _get_skill_or_404(skill_id, app) + repo_path = _governance_repo_path() + try: + return restore_skill_snapshot(repo_path, skill, req.source_ref).to_dict() + except (GitVersionStoreError, ValueError) as e: + raise HTTPException(status_code=400, detail=str(e)) + + def _format_diff(diff: Dict[str, Any]) -> List[Dict[str, Any]]: """将 diff 字典格式化为前端友好的行列表。""" lines = [] @@ -227,3 +362,17 @@ def _format_diff(diff: Dict[str, Any]) -> List[Dict[str, Any]]: "new_lines": str(change).splitlines() or [str(change)], }) return lines + + +async def _get_skill_or_404(skill_id: str, app: AppState): + skill = await app.wiki.get(skill_id) + if not skill: + raise HTTPException(status_code=404, detail=f"Skill {skill_id} not found") + return skill + + +def _governance_repo_path() -> Path: + configured = os.environ.get("SKILLOS_GOVERNANCE_REPO") + if configured: + return Path(configured) + return Path(__file__).resolve().parents[4] diff --git a/skillos/skillos/api/schemas.py b/skillos/skillos/api/schemas.py index 21ac29b..9aebf49 100644 --- a/skillos/skillos/api/schemas.py +++ b/skillos/skillos/api/schemas.py @@ -115,6 +115,44 @@ class NewVersionRequest(BaseModel): author: str = "api" +class SnapshotCommitRequest(BaseModel): + author: str = "api" + message: Optional[str] = None + + +class SnapshotCommitResponse(BaseModel): + skill_id: str + skill_name: str + version: str + snapshot_path: str + commit: str + message: str + + +class SnapshotHistoryResponse(BaseModel): + skill_id: str + snapshot_path: str + history: List[Dict[str, Any]] + + +class SnapshotDiffResponse(BaseModel): + skill_id: str + snapshot_path: str + from_ref: str + to_ref: str + raw_diff: str + diffs: List[Dict[str, Any]] + has_breaking_changes: bool + + +class ReleaseTagRequest(BaseModel): + ref: str = "HEAD" + + +class RollbackRequest(BaseModel): + source_ref: str + + # ─── 图谱 ───────────────────────────────────────────────────────────────────── class AddEdgeRequest(BaseModel): diff --git a/skillos/tests/test_skill_governance_lifecycle_api.py b/skillos/tests/test_skill_governance_lifecycle_api.py new file mode 100644 index 0000000..be0623b --- /dev/null +++ b/skillos/tests/test_skill_governance_lifecycle_api.py @@ -0,0 +1,224 @@ +from __future__ import annotations + +import subprocess +from pathlib import Path +from types import SimpleNamespace + +import pytest +from fastapi import FastAPI +from fastapi.testclient import TestClient + +from skillos.api.deps import get_app_state +from skillos.api.routes.lifecycle import router +from skillos.layers.skill_governance import GitVersionStore, skill_snapshot_path, write_skill_snapshot +from skillos.models import Skill, SkillImplementation, SkillInterface, SkillState, SkillType + + +def _run_git(repo: Path, args: list[str]) -> None: + subprocess.run( + ["git", *args], + cwd=repo, + check=True, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + ) + + +class FakeWiki: + def __init__(self, skill: Skill | None) -> None: + self.skill = skill + + async def get(self, skill_id: str) -> Skill | None: + if self.skill and self.skill.skill_id == skill_id: + return self.skill + return None + + +@pytest.fixture +def git_repo(tmp_path: Path) -> Path: + _run_git(tmp_path, ["init"]) + baseline = tmp_path / ".gitkeep" + baseline.write_text("", encoding="utf-8") + GitVersionStore(tmp_path).commit_paths([".gitkeep"], "initial baseline") + return tmp_path + + +@pytest.fixture +def skill() -> Skill: + return Skill( + skill_id="skill-1234567890", + name="search_wiki", + version="1.0.0", + description="Search wiki entries.", + skill_type=SkillType.FUNCTIONAL, + domain="wiki", + state=SkillState.RELEASED, + tags=["wiki", "search"], + interface=SkillInterface( + input_schema={ + "type": "object", + "properties": {"query": {"type": "string"}}, + "required": ["query"], + }, + output_schema={ + "type": "object", + "properties": {"results": {"type": "array"}}, + }, + ), + implementation=SkillImplementation(prompt_template="Search for {query}"), + ) + + +@pytest.fixture +def client(git_repo: Path, skill: Skill, monkeypatch: pytest.MonkeyPatch) -> TestClient: + monkeypatch.setenv("SKILLOS_GOVERNANCE_REPO", str(git_repo)) + app = FastAPI() + app.include_router(router, prefix="/api/v1") + app.dependency_overrides[get_app_state] = lambda: SimpleNamespace(wiki=FakeWiki(skill)) + return TestClient(app) + + +def _commit_snapshot(repo: Path, skill: Skill, message: str) -> str: + snapshot_path = write_skill_snapshot(repo, skill) + return GitVersionStore(repo).commit_paths([snapshot_path], message) + + +def test_snapshot_endpoint_creates_commit(client: TestClient, git_repo: Path, skill: Skill) -> None: + response = client.post(f"/api/v1/lifecycle/{skill.skill_id}/snapshot", json={}) + + assert response.status_code == 200 + data = response.json() + assert data["snapshot_path"] == skill_snapshot_path(skill) + assert data["message"] == "skill(search_wiki): snapshot v1.0.0" + assert len(data["commit"]) == 40 + assert (git_repo / skill_snapshot_path(skill)).exists() + + +def test_snapshot_history_endpoint_returns_git_history( + client: TestClient, + git_repo: Path, + skill: Skill, +) -> None: + client.post(f"/api/v1/lifecycle/{skill.skill_id}/snapshot", json={"message": "snapshot one"}) + + response = client.get(f"/api/v1/lifecycle/{skill.skill_id}/snapshot/history") + + assert response.status_code == 200 + data = response.json() + assert data["snapshot_path"] == skill_snapshot_path(skill) + assert data["history"][0]["subject"] == "snapshot one" + + +def test_snapshot_diff_endpoint_returns_raw_and_structured_diff( + client: TestClient, + git_repo: Path, + skill: Skill, +) -> None: + first_commit = _commit_snapshot(git_repo, skill, "snapshot original") + skill.description = "Search wiki entries with filters." + second_commit = _commit_snapshot(git_repo, skill, "snapshot changed") + + response = client.get( + f"/api/v1/lifecycle/{skill.skill_id}/snapshot/diff", + params={"from_ref": first_commit, "to_ref": second_commit}, + ) + + assert response.status_code == 200 + data = response.json() + assert "Search wiki entries with filters." in data["raw_diff"] + assert data["diffs"][0]["field"] == "description" + assert data["has_breaking_changes"] is False + + +def test_snapshot_diff_marks_breaking_schema_change( + client: TestClient, + git_repo: Path, + skill: Skill, +) -> None: + first_commit = _commit_snapshot(git_repo, skill, "snapshot original") + skill.interface.input_schema = { + "type": "object", + "properties": {"query": {"type": "integer"}}, + "required": ["query"], + } + second_commit = _commit_snapshot(git_repo, skill, "snapshot breaking") + + response = client.get( + f"/api/v1/lifecycle/{skill.skill_id}/snapshot/diff", + params={"from_ref": first_commit, "to_ref": second_commit}, + ) + + assert response.status_code == 200 + assert response.json()["has_breaking_changes"] is True + + +def test_release_tag_endpoint_creates_tag(client: TestClient, git_repo: Path, skill: Skill) -> None: + commit_hash = _commit_snapshot(git_repo, skill, "snapshot release") + + response = client.post( + f"/api/v1/lifecycle/{skill.skill_id}/release-tag", + json={"ref": commit_hash}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["tag_name"] == "skill/search_wiki/skill-12/v1.0.0" + assert GitVersionStore(git_repo).tag_exists(data["tag_name"]) + + +def test_duplicate_release_tag_returns_400(client: TestClient, git_repo: Path, skill: Skill) -> None: + commit_hash = _commit_snapshot(git_repo, skill, "snapshot release") + client.post(f"/api/v1/lifecycle/{skill.skill_id}/release-tag", json={"ref": commit_hash}) + + response = client.post( + f"/api/v1/lifecycle/{skill.skill_id}/release-tag", + json={"ref": commit_hash}, + ) + + assert response.status_code == 400 + assert "already exists" in response.json()["detail"] + + +def test_rollback_endpoint_creates_restore_commit( + client: TestClient, + git_repo: Path, + skill: Skill, +) -> None: + old_commit = _commit_snapshot(git_repo, skill, "snapshot original") + tag_response = client.post( + f"/api/v1/lifecycle/{skill.skill_id}/release-tag", + json={"ref": old_commit}, + ) + old_tag = tag_response.json()["tag_name"] + skill.description = "Changed description." + _commit_snapshot(git_repo, skill, "snapshot changed") + + response = client.post( + f"/api/v1/lifecycle/{skill.skill_id}/rollback", + json={"source_ref": old_tag}, + ) + + assert response.status_code == 200 + data = response.json() + assert data["commit_message"] == f"skill(search_wiki): restore from {old_tag}" + history = GitVersionStore(git_repo).commit_history(skill_snapshot_path(skill)) + assert history[0].subject == data["commit_message"] + assert any(item.commit_hash == old_commit for item in history) + + +def test_missing_skill_returns_404(client: TestClient) -> None: + response = client.post("/api/v1/lifecycle/missing/snapshot", json={}) + + assert response.status_code == 404 + + +def test_missing_ref_returns_400(client: TestClient, skill: Skill) -> None: + response = client.post( + f"/api/v1/lifecycle/{skill.skill_id}/rollback", + json={"source_ref": "missing-ref"}, + ) + + assert response.status_code == 400 + assert "Git command failed" in response.json()["detail"] From 35f4cb8701f89a711231b88ed95caf0d2206918e Mon Sep 17 00:00:00 2001 From: DYD Date: Mon, 4 May 2026 00:23:24 +0800 Subject: [PATCH 6/6] fix: harden skill snapshot paths --- .../layers/skill_governance/skill_snapshot.py | 18 +++++++++++++-- .../test_skill_governance_snapshot_diff.py | 22 +++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/skillos/skillos/layers/skill_governance/skill_snapshot.py b/skillos/skillos/layers/skill_governance/skill_snapshot.py index 85f50c1..a1ac862 100644 --- a/skillos/skillos/layers/skill_governance/skill_snapshot.py +++ b/skillos/skillos/layers/skill_governance/skill_snapshot.py @@ -3,6 +3,7 @@ from __future__ import annotations import json +import re from dataclasses import dataclass from pathlib import Path from typing import Any, Dict, List, Mapping, Optional @@ -43,6 +44,8 @@ "implementation.sub_skill_ids", ) +SAFE_PATH_COMPONENT = re.compile(r"^[A-Za-z0-9._-]+$") + @dataclass(frozen=True) class SkillSnapshotDiff: @@ -66,7 +69,9 @@ def to_dict(self) -> Dict[str, Any]: def skill_snapshot_path(skill: Skill) -> str: """Return the repo-relative snapshot path for a Skill version.""" - return f"skills/{skill.skill_id}/{skill.version}.json" + skill_id = _safe_snapshot_component(skill.skill_id, "skill_id") + version = _safe_snapshot_component(skill.version, "version") + return f"skills/{skill_id}/{version}.json" def skill_to_snapshot(skill: Skill) -> Dict[str, Any]: @@ -88,7 +93,10 @@ def skill_to_snapshot_json(skill: Skill) -> str: def write_skill_snapshot(repo_path: str | Path, skill: Skill) -> str: """Write a Skill snapshot under the repo path and return its relative path.""" relative_path = skill_snapshot_path(skill) - target = Path(repo_path) / relative_path + repo_root = Path(repo_path).resolve() + target = (repo_root / relative_path).resolve() + if not target.is_relative_to(repo_root): + raise ValueError("Skill snapshot path must stay inside the repository.") target.parent.mkdir(parents=True, exist_ok=True) target.write_text(skill_to_snapshot_json(skill), encoding="utf-8") return relative_path @@ -129,6 +137,12 @@ def has_breaking_changes(diffs: List[SkillSnapshotDiff]) -> bool: return any(diff.is_breaking for diff in diffs) +def _safe_snapshot_component(value: str, field_name: str) -> str: + if not value or not SAFE_PATH_COMPONENT.fullmatch(value) or ".." in value: + raise ValueError(f"Invalid Skill snapshot {field_name}: {value!r}") + return value + + def _coerce_snapshot(snapshot: Mapping[str, Any] | str) -> Dict[str, Any]: if isinstance(snapshot, str): return json.loads(snapshot) diff --git a/skillos/tests/test_skill_governance_snapshot_diff.py b/skillos/tests/test_skill_governance_snapshot_diff.py index 7ca4f0d..698646f 100644 --- a/skillos/tests/test_skill_governance_snapshot_diff.py +++ b/skillos/tests/test_skill_governance_snapshot_diff.py @@ -3,6 +3,8 @@ import subprocess from pathlib import Path +import pytest + from skillos.layers.skill_governance import ( GitVersionStore, commit_skill_snapshot, @@ -11,6 +13,7 @@ skill_snapshot_path, skill_to_snapshot, skill_to_snapshot_json, + write_skill_snapshot, ) from skillos.models import Skill, SkillImplementation, SkillInterface, SkillState, SkillType @@ -98,6 +101,25 @@ def test_snapshot_path_uses_skill_id_and_version() -> None: assert skill_snapshot_path(skill) == "skills/skill-123/1.0.0.json" +def test_snapshot_path_rejects_unsafe_skill_id() -> None: + for unsafe_id in ("../outside", "bad/id", "bad\\id", "bad id"): + skill = _make_skill() + skill.skill_id = unsafe_id + + with pytest.raises(ValueError, match="Invalid Skill snapshot skill_id"): + skill_snapshot_path(skill) + + +def test_write_snapshot_rejects_unsafe_path_before_writing(tmp_path: Path) -> None: + skill = _make_skill() + skill.skill_id = "../outside" + + with pytest.raises(ValueError, match="Invalid Skill snapshot skill_id"): + write_skill_snapshot(tmp_path, skill) + + assert not (tmp_path.parent / "outside").exists() + + def test_description_change_is_not_breaking() -> None: old_snapshot = _snapshot_with() new_snapshot = _snapshot_with(description="Search wiki entries with filters.")