From 7408c532f095c25e0caf6541669d19db9243fcf8 Mon Sep 17 00:00:00 2001 From: csh Date: Thu, 20 Aug 2026 15:21:37 +0800 Subject: [PATCH] :bug: fix(commit-message): enforce one rule owner across skill and CI CI ran .gitea/ci/commit_message_lint.py, an independent reimplementation that parsed the type/emoji mapping out of docs/common/commit_message.md. Its scope pattern accepted fix(-a), fix(a-), fix(a--b) and fix(_), and it had no subject length check, so the gate that actually blocks merges enforced weaker rules than the Validation this skill reports. The CI entry is now a wrapper that locates the skill validator and delegates to it with no arguments; commit_policy.json becomes the only rule source and explanatory docs stop being a machine policy input. The validator also learns the workflow_run event, whose payload carries neither a PR title nor a before/after range. An upstream pull_request now validates ..head_sha instead of HEAD alone, taking the branch name from COMMIT_LINT_MAIN_BRANCH, and degrades to the upstream head commit with a WARN rather than guessing a base. Alongside: --help now documents the no-argument CI mode it had always supported silently, CI wiring detail moves to references/ci-wiring.md, and the description gains negative boundaries. test/test_commit_message_policy.py asserts policy/spec-table equality and uses ast to assert the CI entry imports no regex and reads no file, so a second implementation cannot reappear unnoticed. Co-Authored-By: Claude Fable 5 --- .gitea/ci/commit_message_lint.py | 210 ++++-------------- skills/commit-message/SKILL.md | 30 ++- skills/commit-message/references/ci-wiring.md | 73 ++++++ .../scripts/validate_commit_message.py | 166 ++++++++++++-- skills/commit-message/test-prompts.json | 5 + test/test_commit_message_policy.py | 200 +++++++++++++++++ 6 files changed, 476 insertions(+), 208 deletions(-) create mode 100644 skills/commit-message/references/ci-wiring.md create mode 100644 test/test_commit_message_policy.py diff --git a/.gitea/ci/commit_message_lint.py b/.gitea/ci/commit_message_lint.py index 6baa5386..fe403375 100644 --- a/.gitea/ci/commit_message_lint.py +++ b/.gitea/ci/commit_message_lint.py @@ -1,162 +1,59 @@ #!/usr/bin/env python3 +"""CI entry point for commit-message lint. + +This file only locates and delegates. Every rule, the type/emoji mapping and +the policy schema belong to the commit-message skill validator, which reads +`skills/commit-message/references/commit_policy.json`. + +Do not reimplement validation here and do not parse rules out of +`docs/common/commit_message.md`: that document is human guidance, not a machine +policy source. See `skills/commit-message/references/ci-wiring.md`. +""" from __future__ import annotations -import json import os import pathlib -import re import subprocess import sys -from typing import Dict, List, Optional, Tuple + +VALIDATOR_CANDIDATES = ( + ("skills", "commit-message", "scripts", "validate_commit_message.py"), + ( + "docs", + "standards", + "playbook", + "skills", + "commit-message", + "scripts", + "validate_commit_message.py", + ), +) def _eprint(*args: object) -> None: print(*args, file=sys.stderr) -def _git(*args: str) -> str: - return subprocess.check_output(["git", *args], text=True).strip() - - def _repo_root() -> pathlib.Path: - return pathlib.Path(_git("rev-parse", "--show-toplevel")) + output = subprocess.check_output( + ["git", "rev-parse", "--show-toplevel"], text=True + ) + return pathlib.Path(output.strip()) -def _find_commit_spec(root: pathlib.Path) -> pathlib.Path: - candidates = [ - root / "docs" / "common" / "commit_message.md", - root / "docs" / "standards" / "playbook" / "docs" / "common" / "commit_message.md", - ] - for path in candidates: +def _find_validator(root: pathlib.Path) -> pathlib.Path: + tried: list[pathlib.Path] = [] + for parts in VALIDATOR_CANDIDATES: + path = root.joinpath(*parts) + tried.append(path) if path.is_file(): return path raise FileNotFoundError( - "commit_message.md not found; expected one of:\n" - + "\n".join(f"- {p}" for p in candidates) + "commit-message skill validator not found; expected one of:\n" + + "\n".join(f"- {path}" for path in tried) ) -def _parse_type_emoji_mapping(md_text: str) -> Dict[str, str]: - mapping: Dict[str, str] = {} - for raw_line in md_text.splitlines(): - line = raw_line.strip() - if not (line.startswith("|") and line.endswith("|")): - continue - if "type" in line and "emoji" in line: - continue - if re.fullmatch(r"\|\s*-+\s*(\|\s*-+\s*)+\|", line): - continue - - cols = [c.strip() for c in line.strip("|").split("|")] - if len(cols) < 2: - continue - - m_type = re.search(r"`([^`]+)`", cols[0]) - m_emoji = re.search(r"`(:[^`]+:)`", cols[1]) - if not m_type or not m_emoji: - continue - - type_name = m_type.group(1).strip() - emoji_code = m_emoji.group(1).strip() - mapping[type_name] = emoji_code - - if not mapping: - raise ValueError("failed to parse type/emoji mapping from commit_message.md") - return mapping - - -def _validate_subject_line( - line: str, - mapping: Dict[str, str], - *, - require_emoji: bool, -) -> Optional[str]: - subject = line.strip() - if not subject: - return "empty subject" - - m = re.match( - r"^(?:(?P:[a-z0-9_+-]+:)\s+)?" - r"(?P[a-z]+)" - r"(?P\([a-z0-9_-]+\))?" - r":\s+(?P.+)$", - subject, - ) - if not m: - return "does not match ':emoji: type(scope): subject' or 'type(scope): subject'" - - emoji = m.group("emoji") - type_name = m.group("type") - text = (m.group("text") or "").rstrip() - - if type_name not in mapping: - return f"unknown type: {type_name}" - - if emoji: - expected = mapping[type_name] - if emoji != expected: - return f"emoji/type mismatch: got {emoji} {type_name}, expected {expected} for type {type_name}" - elif require_emoji: - return "missing emoji (set COMMIT_LINT_REQUIRE_EMOJI=0 to allow)" - - if text.endswith((".", "。")): - return "subject should not end with a period" - - return None - - -def _load_event_payload() -> Tuple[str, Optional[dict]]: - event_name = os.getenv("GITHUB_EVENT_NAME") or os.getenv("GITEA_EVENT_NAME") or "" - event_path = os.getenv("GITHUB_EVENT_PATH") or os.getenv("GITEA_EVENT_PATH") or "" - if not event_path: - return event_name, None - - path = pathlib.Path(event_path) - if not path.is_file(): - return event_name, None - - try: - return event_name, json.loads(path.read_text(encoding="utf-8")) - except Exception as exc: - _eprint(f"WARN: failed to parse event payload: {path} ({exc})") - return event_name, None - - -def _gather_subjects(event_name: str, payload: Optional[dict]) -> List[Tuple[str, str]]: - subjects: List[Tuple[str, str]] = [] - - if isinstance(payload, dict): - if event_name.startswith("pull_request"): - pr = payload.get("pull_request") - if isinstance(pr, dict): - title = (pr.get("title") or "").strip() - if title: - subjects.append(("pull_request.title", title.splitlines()[0].strip())) - - if event_name == "push": - commits = payload.get("commits") - if isinstance(commits, list): - for commit in commits: - if not isinstance(commit, dict): - continue - msg = (commit.get("message") or "").strip() - if not msg: - continue - subject = msg.splitlines()[0].strip() - sha = commit.get("id") or commit.get("sha") or "" - label = f"push.commit {sha[:7]}" if sha else "push.commit" - subjects.append((label, subject)) - - if subjects: - return subjects - - try: - subjects.append(("HEAD", _git("log", "-1", "--format=%s", "HEAD"))) - except Exception: - pass - return subjects - - def main() -> int: try: root = _repo_root() @@ -166,45 +63,18 @@ def main() -> int: os.chdir(root) - require_emoji = os.getenv("COMMIT_LINT_REQUIRE_EMOJI", "1") not in ("0", "false", "False") - try: - spec_path = _find_commit_spec(root) + validator = _find_validator(root) except FileNotFoundError as exc: _eprint(f"ERROR: {exc}") return 2 - try: - mapping = _parse_type_emoji_mapping(spec_path.read_text(encoding="utf-8")) - except Exception as exc: - _eprint(f"ERROR: failed to read/parse {spec_path}: {exc}") - return 2 - - event_name, payload = _load_event_payload() - subjects = _gather_subjects(event_name, payload) - - print(f"commit spec: {spec_path}") - if event_name: - print(f"event: {event_name}") - print(f"require emoji: {require_emoji}") - print(f"checks: {len(subjects)} subject(s)") - - errors: List[str] = [] - for label, subject in subjects: - err = _validate_subject_line(subject, mapping, require_emoji=require_emoji) - if err: - errors.append(f"- {label}: {err}\n subject: {subject}") - - if errors: - _eprint("ERROR: commit message lint failed:") - for item in errors: - _eprint(item) - return 1 - - print("OK") - return 0 + print(f"validator: {validator}") + sys.stdout.flush() + # No arguments: the validator selects its CI input mode from the event + # payload. Exit codes pass through unchanged (0 pass / 1 lint / 2 input). + return subprocess.run([sys.executable, str(validator)], cwd=root).returncode if __name__ == "__main__": raise SystemExit(main()) - diff --git a/skills/commit-message/SKILL.md b/skills/commit-message/SKILL.md index 35d6a1dc..d221335a 100644 --- a/skills/commit-message/SKILL.md +++ b/skills/commit-message/SKILL.md @@ -1,6 +1,6 @@ --- name: commit-message -description: "当用户需要撰写或审查提交信息、检查已暂存改动、判断是否拆分提交，或使用 emoji 与提交类型（作用域）格式时使用。" +description: "当用户需要撰写或审查提交信息、检查已暂存改动、判断是否拆分提交，或使用 emoji 与提交类型（作用域）格式时使用。不用于交互式起草 PR 标题、发布说明或变更日志，不用于照原文执行用户已经给定的提交，也不用于诊断 CI 为什么失败（改用 gitea-fix-ci）。" --- # Commit Message（提交信息） @@ -14,10 +14,14 @@ description: "当用户需要撰写或审查提交信息、检查已暂存改动 - `references/commit_policy.json`：提交信息格式策略。 - `scripts/validate_commit_message.py`：候选信息和 CI 输入校验器。 +- `references/ci-wiring.md`：把 validator 接到 CI 的方式与事件覆盖范围。 以包含本文件的目录作为 skill 根目录解析上述路径。不要假设存在 Playbook checkout、 仓库 `docs/` 目录或特定的当前工作目录。 +validator 是提交信息规则的唯一实现。CI 入口只能调用它，不得自行实现校验，也不得 +从说明性文档解析 type/emoji 等规则；说明性文档是给人读的，不是机器策略来源。 + ## 适用场景 以下情况使用本 skill：撰写提交信息、审查已暂存边界、判断是否拆分提交，或处理 @@ -110,24 +114,16 @@ policy 的 `emoji.requirement_env` 未设置时使用 `required_by_default`； index 也不等于授权提交。未获得相应授权时，不运行 `git commit`，不暂存文件， 不修改 index。 -## CI 输入校验（仅无参数模式） +## CI 输入校验 -无参数运行 validator 时检查 CI payload；只有未识别事件或本地调用才回退 `HEAD`。 -已识别的 push/PR 事件缺失或损坏 payload 时必须失败。 +无参数运行 validator 即进入 CI 输入校验：从事件 payload 取回全部待校验主题，只有 +未识别事件或本地调用才回退 `HEAD`。已识别事件缺失或损坏 payload 时必须失败； +shallow repository 一律失败；不能用可能截断的 payload `commits` 数组或计数字段 +证明范围完整。 -- **Pull request**：同时校验 `pull_request.title` 和本地 Git 中 - `base.sha..head.sha` 的完整提交范围；遇到 shallow repository、缺失范围元数据或 - Git 对象时失败。workflow 编排必须来自可信 base/default branch：直接触发使用 - `pull_request_target`，或先由 `pull_request` 准备输入、再由默认分支的 - `workflow_run` 执行。普通 `pull_request` 中来自 PR head 的 workflow 即使切到 base - worktree，仍可被待审改动删除或绕过。可信 workflow 必须从 `base.sha` 的可信 base - worktree 运行 wrapper、validator 和 policy；只能读取 PR head 的 Git 对象，不能在 - 持有 secret 的步骤 checkout 或执行 PR head 内容。 -- **Push**：校验本地 Git 中 `before..after` 的完整范围。新分支 push 的 `before` 为 - 零对象时，使用 payload 的目标 `refs/heads/*` 和本地分支图计算该分支相对其它分支 - 新增的完整提交集合。缺少有效范围元数据、目标引用或所需 Git 对象时失败；shallow - repository 也必须失败。不能用可能截断的 payload `commits` 数组或计数字段证明 - 范围完整。 +覆盖哪些事件、每种事件取回什么范围、以及可信编排要求见 +`references/ci-wiring.md`。接线或排查前先运行 +`validate_commit_message.py --help`，以脚本当前输出为事件覆盖范围的权威。 ## 输出约定 diff --git a/skills/commit-message/references/ci-wiring.md b/skills/commit-message/references/ci-wiring.md new file mode 100644 index 00000000..83110e83 --- /dev/null +++ b/skills/commit-message/references/ci-wiring.md @@ -0,0 +1,73 @@ +# CI 接线（CI Wiring） + +配置或排查 CI 上的提交信息校验时读本文件。交互式起草提交信息不需要它。 + +## 唯一实现 + +`scripts/validate_commit_message.py` 是提交信息规则的唯一实现，策略来自 +`references/commit_policy.json`。CI 入口只负责定位并调用它： + +```bash +python /scripts/validate_commit_message.py +``` + +无参数即进入 CI 输入校验模式。退出码原样透传：`0` 全部通过、`1` 存在不合规主题、 +`2` 策略/参数/输入错误。 + +CI 入口不得自行实现校验，不得从 `docs/common/commit_message.md` 之类的说明性文档 +解析 type/emoji 映射。说明性文档是给人读的；把它当机器策略来源会让 CI 与 +`commit_policy.json` 各自演进，而 validator 报告的 `Validation` 就不再代表实际 +拦合并的那道门。本仓库的 `.gitea/ci/commit_message_lint.py` 是薄 wrapper 范例， +其纪律由 `test/test_commit_message_policy.py` 机器保证。 + +## 事件覆盖范围 + +以 `validate_commit_message.py --help` 的当前输出为权威；下表是形状说明。 + +| 事件 | 取回的主题 | +| --------------- | ----------------------------------------------------------------- | +| `pull_request*` | `pull_request.title` 加 `base.sha..head.sha` 的完整提交范围 | +| `push` | `before..after` 完整范围；`before` 为零对象时按分支图计算新增提交 | +| `workflow_run` | 见下节 | +| 其它 / 本地 | 回退 `HEAD` 一条 | + +已识别事件缺失或损坏 payload 时失败；shallow repository 一律失败。不能用可能截断的 +payload `commits` 数组或计数字段证明范围完整。 + +## workflow_run 编排 + +`workflow_run` payload 不含 PR 标题，也不含 `before`/`after`，`workflow_run.head_sha` +是唯一锚点。因此： + +- **上游为 `pull_request*`**：取 `<集成分支>..head_sha` 的完整范围。集成分支名读 + `$COMMIT_LINT_MAIN_BRANCH`（默认 `main`），依次尝试 + `refs/remotes/origin/<名字>` 与 `refs/heads/<名字>`。 +- **其它上游事件**：只校验 `head_sha` 一条。 +- 集成分支无法解析、上游 `head_branch` 就是集成分支、或计算出的范围为空时，退化为 + 只校验 `head_sha` 并打印 `WARN`，**不猜测 base**。默认分支的 push 因此只校验被推 + 上去的那一条，这是可接受的：进入默认分支的路径由 PR 那道门守。 + +该架构下 PR 标题不参与校验，这是 payload 决定的，不是配置项。完整提交范围校验取代 +它，且强度更高——标题一直只是历史的代理，范围校验管的是真正留在历史里的东西。若确实 +需要校验 PR 标题，只能在直接响应 `pull_request` 的那个 workflow 里做。 + +## 可信编排 + +workflow 编排必须来自可信 base/default branch。两条路： + +1. 直接触发用 `pull_request_target`。 +2. 先由 `pull_request` 准备输入，再由默认分支的 `workflow_run` 执行。 + +普通 `pull_request` 中来自 PR head 的 workflow 即使切到 base worktree，仍可被待审改动 +删除或绕过。可信 workflow 必须从可信 base 运行 wrapper、validator 和 policy；只能读取 +PR head 的 Git 对象，不能在持有 secret 的步骤 checkout 或执行 PR head 内容。 + +## 排查 + +| 现象 | 处理 | +| ----------------------------------------- | ------------------------------------------------------------- | +| `rc=2` 且 `ERROR: ...policy...` | 部署或配置问题，不是提交信息不合规；检查 policy 路径与 schema | +| `rc=2` 且 `ERROR: ...payload...` | 事件 payload 缺失或损坏；核对 `GITHUB_EVENT_PATH` | +| `rc=2` 且 `requires complete ... history` | checkout 是 shallow；改为完整 fetch | +| `checks: 1 subject(s)` 但期望是一个范围 | 读 stderr 的 `WARN`：集成分支未解析，或上游不是 PR | +| `validator: ... not found` | wrapper 的候选路径与实际部署布局不符 | diff --git a/skills/commit-message/scripts/validate_commit_message.py b/skills/commit-message/scripts/validate_commit_message.py index 921b217d..fe8b968f 100644 --- a/skills/commit-message/scripts/validate_commit_message.py +++ b/skills/commit-message/scripts/validate_commit_message.py @@ -21,6 +21,38 @@ HEADER_RE = re.compile( r":\s+(?P.+)$" ) +HELP_EPILOG = """\ +三种模式（互斥；未给出 --subject / --message-file 时进入 CI 输入校验）: + + --subject "<候选>" 校验单个候选主题；交互式起草提交信息时用这一种 + --message-file 校验提交信息文件的首行；body/footer 不做机械校验 + (无参数) CI 输入校验：从事件 payload 取回全部待校验主题 + +策略来源固定优先级：--policy > $COMMIT_POLICY_PATH > 本脚本旁的 +references/commit_policy.json。策略缺失、损坏或 schema_version 不支持时报部署 +错误，不退回任何内置约定。本脚本是提交信息规则的唯一实现；说明性文档不是机器 +策略来源。 + +CI 输入校验按事件展开： + + pull_request* pull_request.title 加 base.sha..head.sha 的完整提交范围 + push before..after 完整范围；before 为零对象时按分支图计算新增提交 + workflow_run 上游为 pull_request* 时，取 <集成分支>..workflow_run.head_sha + 的完整范围；其它上游事件只校验 workflow_run.head_sha 一条 + 其它 / 本地 回退 HEAD 一条 + +集成分支名取 $COMMIT_LINT_MAIN_BRANCH，默认 main；依次尝试 +refs/remotes/origin/<名字> 与 refs/heads/<名字>。无法解析、或上游 head_branch +就是集成分支时，退化为只校验上游 head 一条并打印 WARN，不猜测 base。 + +workflow_run payload 不含 PR 标题，该架构下标题不参与校验；完整提交范围校验取代 +它，且强度更高。 + +已识别事件缺失或损坏 payload 时失败；shallow repository 一律失败。 + +退出码：0 全部通过；1 存在不合规主题；2 策略、参数或输入错误。 +""" + class PolicyError(ValueError): pass @@ -233,7 +265,11 @@ def _read_message_file(path: Path) -> str: def _parse_args(argv: list[str] | None) -> argparse.Namespace: - parser = argparse.ArgumentParser(description="Validate commit-message subjects") + parser = argparse.ArgumentParser( + description="Validate commit-message subjects", + epilog=HELP_EPILOG, + formatter_class=argparse.RawDescriptionHelpFormatter, + ) parser.add_argument("--policy", help="path to a versioned commit policy JSON") candidates = parser.add_mutually_exclusive_group() candidates.add_argument("--subject", help="single commit subject to validate") @@ -246,7 +282,11 @@ def _parse_args(argv: list[str] | None) -> argparse.Namespace: def _requires_event_payload(event_name: str) -> bool: - return event_name == "push" or event_name.startswith("pull_request") + return ( + event_name == "push" + or event_name == "workflow_run" + or event_name.startswith("pull_request") + ) def _load_event_payload() -> tuple[str, dict[str, object] | None]: @@ -335,19 +375,9 @@ def _require_complete_git_history(context: str) -> None: raise InputError(f"cannot determine repository depth for {context}") -def _zero_before_subjects( - payload: dict[str, object], after: str -) -> list[tuple[str, str]]: - target_ref = payload.get("ref") - if not isinstance(target_ref, str) or not target_ref.startswith("refs/heads/"): - raise InputError("zero-before push event requires a refs/heads/* ref") +def _other_branch_refs(branch_name: str, context: str) -> list[str]: + """Non-symbolic branch refs that are not `branch_name` on any remote.""" try: - check_ref = subprocess.run( - ["git", "check-ref-format", target_ref], - capture_output=True, - text=True, - check=False, - ) refs_result = subprocess.run( [ "git", @@ -361,29 +391,50 @@ def _zero_before_subjects( check=False, ) except OSError as exc: - raise InputError(f"cannot inspect refs for zero-before push: {exc}") from exc - if check_ref.returncode != 0: - raise InputError(f"invalid zero-before push ref: {target_ref}") + raise InputError(f"cannot inspect refs for {context}: {exc}") from exc if refs_result.returncode != 0: detail = refs_result.stderr.strip() suffix = f": {detail}" if detail else "" - raise InputError(f"cannot inspect refs for zero-before push{suffix}") + raise InputError(f"cannot inspect refs for {context}{suffix}") - branch_name = target_ref.removeprefix("refs/heads/") + own_ref = f"refs/heads/{branch_name}" exclusions: list[str] = [] for line in refs_result.stdout.splitlines(): ref_name, separator, symbolic_target = line.partition("\0") if not separator or not ref_name: - raise InputError("invalid git ref output for zero-before push") + raise InputError(f"invalid git ref output for {context}") if symbolic_target: continue - if ref_name == target_ref: + if ref_name == own_ref: continue if ref_name.startswith("refs/remotes/"): remote_parts = ref_name.split("/", 3) if len(remote_parts) == 4 and remote_parts[3] == branch_name: continue exclusions.append(ref_name) + return exclusions + + +def _zero_before_subjects( + payload: dict[str, object], after: str +) -> list[tuple[str, str]]: + target_ref = payload.get("ref") + if not isinstance(target_ref, str) or not target_ref.startswith("refs/heads/"): + raise InputError("zero-before push event requires a refs/heads/* ref") + try: + check_ref = subprocess.run( + ["git", "check-ref-format", target_ref], + capture_output=True, + text=True, + check=False, + ) + except OSError as exc: + raise InputError(f"cannot inspect refs for zero-before push: {exc}") from exc + if check_ref.returncode != 0: + raise InputError(f"invalid zero-before push ref: {target_ref}") + + branch_name = target_ref.removeprefix("refs/heads/") + exclusions = _other_branch_refs(branch_name, "zero-before push") revisions = [after] if exclusions: @@ -417,6 +468,73 @@ def _push_range_subjects(payload: dict[str, object]) -> list[tuple[str, str]]: ) +def _integration_branch() -> tuple[str, str | None]: + """The configured integration branch name and its first resolvable ref.""" + name = (os.getenv("COMMIT_LINT_MAIN_BRANCH") or "").strip() or "main" + for ref in (f"refs/remotes/origin/{name}", f"refs/heads/{name}"): + try: + result = subprocess.run( + ["git", "rev-parse", "--verify", "--quiet", ref], + capture_output=True, + text=True, + check=False, + ) + except OSError as exc: + raise InputError(f"cannot resolve {ref}: {exc}") from exc + if result.returncode == 0 and result.stdout.strip(): + return name, ref + return name, None + + +def _workflow_run_subjects(payload: dict[str, object]) -> list[tuple[str, str]]: + """Subjects for a run triggered by another run's completion. + + A workflow_run payload carries no PR title and no before/after range, so the + upstream head SHA is the only anchor. For an upstream pull_request the range + is taken against the integration branch; every other case validates the + upstream head commit alone rather than guessing a base. + """ + run = payload.get("workflow_run") + if not isinstance(run, dict): + raise InputError("workflow_run event payload.workflow_run must be an object") + head_sha = run.get("head_sha") + if not isinstance(head_sha, str) or GIT_OBJECT_ID_RE.fullmatch(head_sha) is None: + raise InputError("workflow_run.head_sha must be a full Git object ID") + upstream_event = run.get("event") + if not isinstance(upstream_event, str) or not upstream_event: + raise InputError("workflow_run.event must be a non-empty string") + _require_complete_git_history("workflow_run event") + + label_prefix = f"workflow_run({upstream_event}).commit" + head_only = f"validating only upstream head commit {head_sha[:7]}" + if upstream_event.startswith("pull_request"): + head_branch = run.get("head_branch") + if not isinstance(head_branch, str) or not head_branch: + raise InputError("workflow_run.head_branch must be a non-empty string") + main_name, main_ref = _integration_branch() + if main_ref is None: + _eprint(f"WARN: no {main_name} ref to compare against; {head_only}") + elif head_branch == main_name: + _eprint(f"WARN: upstream head_branch is {main_name}; {head_only}") + else: + revision_range = f"{main_ref}..{head_sha}" + subjects = _git_log_subjects( + [head_sha, "--not", main_ref], + f"workflow_run pull-request range {revision_range}", + allow_empty=True, + label_prefix=label_prefix, + ) + if subjects: + return subjects + _eprint(f"WARN: {revision_range} is empty; {head_only}") + + return _git_log_subjects( + ["-1", head_sha], + f"workflow_run head commit {head_sha[:7]}", + label_prefix=label_prefix, + ) + + def _gather_ci_subjects() -> tuple[str, list[tuple[str, str]]]: event_name, payload = _load_event_payload() subjects: list[tuple[str, str]] = [] @@ -475,8 +593,14 @@ def _gather_ci_subjects() -> tuple[str, list[tuple[str, str]]]: label = f"push.commit {sha_text[:7]}" if sha_text else "push.commit" subjects.append((label, subject)) range_subjects = _push_range_subjects(payload) + # The loop above only asserts payload shape; the Git range is authority. return event_name, range_subjects + if event_name == "workflow_run": + if not isinstance(payload, dict): + raise InputError("workflow_run event payload is required") + return event_name, _workflow_run_subjects(payload) + try: result = subprocess.run( ["git", "log", "-1", "--format=%s", "HEAD"], diff --git a/skills/commit-message/test-prompts.json b/skills/commit-message/test-prompts.json index 34ec92ff..c0b940eb 100644 --- a/skills/commit-message/test-prompts.json +++ b/skills/commit-message/test-prompts.json @@ -13,5 +13,10 @@ "id": "no-staged-diff", "prompt": "帮我根据当前改动写一个规范的 commit message。现在只有未暂存改动，没有 staged diff。", "expected": "检查后明确指出没有已暂存差异，因此停止生成基于实际 staged diff 的最终建议；仅在用户明确要求时才可基于未暂存内容给草稿，不擅自 git add 或 git commit。" + }, + { + "id": "looks-valid-but-policy-rejects", + "prompt": "暂存区只有一处改动。我想用 `:sparkles: feat(order-api-): support batch cancel` 这个提交信息，格式我看着没问题，你确认一下就行。", + "expected": "不以肉眼检查代替校验，仍按 argv 数组调用 bundled validator；validator 因 scope 尾随分隔符报 invalid scope 且 rc=1，据此引用 validator 原文原因、给出修正后的候选并重新校验，不把未校验或已失败的主题写进 Proposed。" } ] diff --git a/test/test_commit_message_policy.py b/test/test_commit_message_policy.py new file mode 100644 index 00000000..716f2713 --- /dev/null +++ b/test/test_commit_message_policy.py @@ -0,0 +1,200 @@ +"""Guards that commit-message rules have exactly one implementation. + +The skill validator owns every rule and reads `references/commit_policy.json`. +`docs/common/commit_message.md` is human guidance; the CI entry point must +delegate rather than re-derive rules from that document. +""" + +import ast +import json +import os +import re +import subprocess +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SKILL_ROOT = ROOT / "skills" / "commit-message" +VALIDATOR = SKILL_ROOT / "scripts" / "validate_commit_message.py" +POLICY = SKILL_ROOT / "references" / "commit_policy.json" +CI_ENTRY = ROOT / ".gitea" / "ci" / "commit_message_lint.py" +SPEC = ROOT / "docs" / "common" / "commit_message.md" + +TABLE_ROW_RE = re.compile(r"^\|(?P[^|]*)\|(?P[^|]*)\|") + + +def run(script, *args, env=None): + merged = {**os.environ, **(env or {})} + merged.pop("COMMIT_POLICY_PATH", None) + return subprocess.run( + [sys.executable, str(script), *args], + cwd=ROOT, + capture_output=True, + text=True, + env=merged, + ) + + +def spec_type_emoji_mapping(): + mapping = {} + for line in SPEC.read_text(encoding="utf-8").splitlines(): + match = TABLE_ROW_RE.match(line.strip()) + if not match: + continue + type_cell = re.search(r"`([a-z][a-z0-9-]*)`", match.group("type")) + emoji_cell = re.search(r"`(:[a-z0-9_+-]+:)`", match.group("emoji")) + if type_cell and emoji_cell: + mapping[type_cell.group(1)] = emoji_cell.group(1) + return mapping + + +class CommitPolicySingleOwnerTests(unittest.TestCase): + def test_policy_and_spec_table_agree(self): + policy_types = json.loads(POLICY.read_text(encoding="utf-8"))["types"] + spec_types = spec_type_emoji_mapping() + self.assertTrue(spec_types, f"no type/emoji table parsed from {SPEC}") + self.assertEqual( + policy_types, + spec_types, + "commit_policy.json and docs/common/commit_message.md disagree; " + "update both when adding or renaming a type", + ) + + def test_ci_entry_point_does_not_reimplement_rules(self): + source = CI_ENTRY.read_text(encoding="utf-8") + self.assertIn("validate_commit_message.py", source) + + tree = ast.parse(source) + imported = { + alias.name.split(".")[0] + for node in ast.walk(tree) + if isinstance(node, ast.Import) + for alias in node.names + } | { + node.module.split(".")[0] + for node in ast.walk(tree) + if isinstance(node, ast.ImportFrom) and node.module + } + self.assertNotIn( + "re", + imported, + f"{CI_ENTRY.name} must delegate; a regex means it parses rules itself", + ) + + called = set() + for node in ast.walk(tree): + if not isinstance(node, ast.Call): + continue + function = node.func + if isinstance(function, ast.Attribute): + called.add(function.attr) + elif isinstance(function, ast.Name): + called.add(function.id) + for reader in ("read_text", "open", "read"): + self.assertNotIn( + reader, + called, + f"{CI_ENTRY.name} must not read a rule source; it only delegates", + ) + + def test_ci_entry_point_reports_the_validator_it_delegates_to(self): + result = run(CI_ENTRY) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("validator:", result.stdout) + self.assertIn("validate_commit_message.py", result.stdout) + self.assertIn("commit policy:", result.stdout) + + +class SubjectRuleTests(unittest.TestCase): + def assert_subject(self, subject, expected_rc): + result = run(VALIDATOR, "--subject", subject) + self.assertEqual( + result.returncode, + expected_rc, + f"{subject!r}\nstdout={result.stdout}\nstderr={result.stderr}", + ) + + def test_valid_subjects_pass(self): + for subject in ( + ":bug: fix(core): repair the thing", + ":sparkles: feat(tsl-api-reference): add lookup", + ":wrench: chore(ci_gitea): retune runner", + ":memo: docs: describe the flow", + ): + with self.subTest(subject=subject): + self.assert_subject(subject, 0) + + def test_malformed_scopes_are_rejected(self): + for scope in ("-a", "a-", "a--b", "_", "A", "a b"): + with self.subTest(scope=scope): + self.assert_subject(f":bug: fix({scope}): repair the thing", 1) + + def test_overlong_subject_is_rejected(self): + self.assert_subject(":bug: fix(core): " + "a" * 73, 1) + self.assert_subject(":bug: fix(core): " + "a" * 72, 0) + + def test_period_suffixes_are_rejected(self): + self.assert_subject(":bug: fix(core): repair the thing.", 1) + self.assert_subject(":bug: fix(core): 修好了这个东西。", 1) + + def test_unknown_type_and_emoji_mismatch_are_rejected(self): + self.assert_subject(":bug: nope(core): repair the thing", 1) + self.assert_subject(":memo: fix(core): repair the thing", 1) + + def test_missing_policy_reports_deployment_error(self): + self.assert_no_such_policy() + + def assert_no_such_policy(self): + result = run(VALIDATOR, "--policy", "no/such/policy.json", "--subject", "x") + self.assertEqual(result.returncode, 2, result.stdout) + self.assertIn("ERROR", result.stderr) + + +class WorkflowRunEventTests(unittest.TestCase): + def head_sha(self): + return subprocess.check_output( + ["git", "rev-parse", "HEAD"], cwd=ROOT, text=True + ).strip() + + def run_with_payload(self, payload): + with tempfile.TemporaryDirectory() as directory: + path = Path(directory) / "event.json" + path.write_text(json.dumps(payload), encoding="utf-8") + return run( + VALIDATOR, + env={ + "GITHUB_EVENT_NAME": "workflow_run", + "GITHUB_EVENT_PATH": str(path), + }, + ) + + def test_upstream_push_validates_the_upstream_head_commit(self): + result = self.run_with_payload( + { + "workflow_run": { + "head_sha": self.head_sha(), + "head_branch": "main", + "event": "push", + } + } + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertIn("event: workflow_run", result.stdout) + self.assertIn("workflow_run(push).commit", result.stdout) + self.assertNotIn("input: HEAD", result.stdout) + + def test_malformed_payload_is_an_input_error(self): + for run_payload in ({}, {"head_sha": "nope", "event": "push"}, {"head_sha": "0" * 40}): + with self.subTest(payload=run_payload): + result = self.run_with_payload({"workflow_run": run_payload}) + self.assertEqual(result.returncode, 2, result.stdout) + + def test_missing_payload_path_is_an_input_error(self): + result = run(VALIDATOR, env={"GITHUB_EVENT_NAME": "workflow_run"}) + self.assertEqual(result.returncode, 2, result.stdout) + + +if __name__ == "__main__": + unittest.main()