From 00a08b3e533d19d9e469a1b81129608f78ae442a Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 18:44:25 +0200 Subject: [PATCH 1/8] chore(pre-commit): modular hooks aligned with specfact-cli-modules - Add scripts/pre-commit-quality-checks.sh (block1 stages + block2; all for manual/shim) - Replace monolithic smart-checks with shim to quality-checks all - .pre-commit-config: fail_fast, verify-module-signatures + check-version-sources, cli-block1-* hooks, cli-block2, doc frontmatter - Match modules: hatch run lint when Python staged; scoped code review paths - CLI extras: Markdown fix/lint, workflow actionlint (no packages/ bundle-import gate) - Bump to 0.46.1; docs: README, CONTRIBUTING, code-review.md, agent-rules/70 Made-with: Cursor --- .pre-commit-config.yaml | 71 ++- CHANGELOG.md | 12 + CONTRIBUTING.md | 13 +- README.md | 16 +- .../agent-rules/70-release-commit-and-docs.md | 14 +- docs/modules/code-review.md | 32 +- pyproject.toml | 4 +- scripts/pre-commit-quality-checks.sh | 449 ++++++++++++++++++ scripts/pre-commit-smart-checks.sh | 337 +------------ scripts/setup-git-hooks.sh | 3 + setup.py | 2 +- src/__init__.py | 2 +- src/specfact_cli/__init__.py | 2 +- .../test_pre_commit_smart_checks_docs.py | 37 +- .../test_trustworthy_green_checks.py | 37 +- 15 files changed, 639 insertions(+), 392 deletions(-) create mode 100755 scripts/pre-commit-quality-checks.sh diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 3e1d1bd2..0d94c173 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,12 +1,75 @@ +# Stop after the first failing hook so a broken Block 1 never runs Block 2 (code review + contract tests). +# Layout and stages mirror specfact-cli-modules; CLI adds Markdown + workflow lint + version hook. +fail_fast: true + repos: - repo: local hooks: - - id: specfact-smart-checks - name: SpecFact smart pre-commit checks - entry: scripts/pre-commit-smart-checks.sh - language: script + - id: verify-module-signatures + name: Verify module signatures and version bumps + entry: hatch run ./scripts/verify-modules-signature.py --require-signature --payload-from-filesystem --enforce-version-bump + language: system + pass_filenames: false + always_run: true + + - id: check-version-sources + name: Check synchronized version sources + entry: hatch run check-version-sources + language: system + files: ^(pyproject\.toml|setup\.py|src/__init__\.py|src/specfact_cli/__init__\.py)$ + pass_filenames: false + + - id: cli-block1-format + name: "CLI Block 1 — format" + entry: ./scripts/pre-commit-quality-checks.sh block1-format + language: system + pass_filenames: false + always_run: true + verbose: true + + - id: cli-block1-yaml + name: "CLI Block 1 — yaml-lint (when YAML staged)" + entry: ./scripts/pre-commit-quality-checks.sh block1-yaml + language: system + files: \.(yaml|yml)$ + verbose: true + + - id: cli-block1-markdown-fix + name: "CLI Block 1 — markdown auto-fix (when Markdown staged)" + entry: ./scripts/pre-commit-quality-checks.sh block1-markdown-fix + language: system + files: \.md$ + verbose: true + + - id: cli-block1-markdown-lint + name: "CLI Block 1 — markdownlint (when Markdown staged)" + entry: ./scripts/pre-commit-quality-checks.sh block1-markdown-lint + language: system + files: \.md$ + verbose: true + + - id: cli-block1-workflows + name: "CLI Block 1 — workflow lint (when workflows staged)" + entry: ./scripts/pre-commit-quality-checks.sh block1-workflows + language: system + files: ^\.github/workflows/.*\.(yaml|yml)$ + verbose: true + + - id: cli-block1-lint + name: "CLI Block 1 — lint (when Python staged)" + entry: ./scripts/pre-commit-quality-checks.sh block1-lint + language: system + files: \.(py|pyi)$ + verbose: true + + - id: cli-block2 + name: "CLI Block 2 — code review + contract tests" + entry: ./scripts/pre-commit-quality-checks.sh block2 + language: system pass_filenames: false always_run: true + verbose: true + - id: check-doc-frontmatter name: Check documentation ownership frontmatter (enforced paths) entry: hatch run doc-frontmatter-check diff --git a/CHANGELOG.md b/CHANGELOG.md index 6f83e3af..3032ee08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,18 @@ All notable changes to this project will be documented in this file. --- +## [0.46.1] - 2026-04-14 + +### Changed + +- **Pre-commit**: modular hook layout with `fail_fast` (parity with `specfact-cli-modules`): dedicated + module verify + version-source hook, staged YAML/Markdown/workflow gates, `hatch run lint` when + Python is staged, then Block 2 (scoped code review + contract tests). New + `scripts/pre-commit-quality-checks.sh`; `scripts/pre-commit-smart-checks.sh` remains a shim for + downstream `specfact-smart-checks` consumers. + +--- + ## [0.46.0] - 2026-04-13 ### Added diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index bedc6241..8639464f 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -78,11 +78,22 @@ hatch test --cover -v ### Pre-commit Checks +Local hooks use **`fail_fast: true`** and a **modular layout** aligned with `specfact-cli-modules`: +verify module signatures → sync version files when those paths are staged → format (always) → +YAML / Markdown / workflow lint when matching paths are staged → **`hatch run lint`** when Python +is staged → Block 2 (scoped code review + contract tests, with a safe-change short-circuit for +docs-only and similar commits). See `.pre-commit-config.yaml` and `scripts/pre-commit-quality-checks.sh`. + ```bash -# Install repo hooks +# Install framework hooks (recommended; matches CI-style stages) pre-commit install + +# Optional: copy raw script into .git/hooks/pre-commit (runs full `all` pipeline via shim) scripts/setup-git-hooks.sh +# Manual full pipeline (same as shim) +hatch run pre-commit-checks + # Format code hatch run format diff --git a/README.md b/README.md index 57f844b1..f1239bdd 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ uvx specfact-cli code review run --path . --scope full **Sample output:** ```text -SpecFact CLI - v0.46.0 +SpecFact CLI - v0.46.1 Running Ruff checks... Running Radon complexity checks... @@ -80,16 +80,24 @@ It exists because delivery drifts in predictable ways: ## Add SpecFact to your workflow -**Pre-commit hook** +### Pre-commit + +This repository uses a **modular** local hook layout (parity with `specfact-cli-modules`: `fail_fast`, +separate verify / format / YAML / Markdown / workflow / lint / Block 2 hooks). Copy +[`.pre-commit-config.yaml`](.pre-commit-config.yaml) from this repo and run `pre-commit install`. + +For a **single-hook** setup in downstream repos, keep using the stable id and script shim: ```yaml - repo: https://github.com/nold-ai/specfact-cli - rev: v0.46.0 + rev: v0.46.1 hooks: - id: specfact-smart-checks ``` -**GitHub Actions** +The shim runs `scripts/pre-commit-quality-checks.sh all` (full pipeline including module verify). + +### GitHub Actions ```yaml - name: SpecFact Gate diff --git a/docs/agent-rules/70-release-commit-and-docs.md b/docs/agent-rules/70-release-commit-and-docs.md index 89536027..e0100925 100644 --- a/docs/agent-rules/70-release-commit-and-docs.md +++ b/docs/agent-rules/70-release-commit-and-docs.md @@ -35,33 +35,33 @@ depends_on: - agent-rules-quality-gates-and-review --- -# Agent release, commit, and docs rules +## Agent release, commit, and docs rules -## Versioning +### Versioning - Keep version updates in sync across `pyproject.toml`, `setup.py`, and `src/specfact_cli/__init__.py`. -- **Automated check:** Before tagging or publishing, run `hatch run check-version-sources` (or `python scripts/check_version_sources.py`). It exits non-zero with a clear diff if `pyproject.toml`, `setup.py`, `src/__init__.py`, and `src/specfact_cli/__init__.py` disagree. The **Tests** job in `.github/workflows/pr-orchestrator.yml` runs the same script so mismatches fail CI. Pre-commit runs it whenever a version file is staged (see `scripts/pre-commit-smart-checks.sh`) instead of treating version-only commits as “safe” without verification. +- **Automated check:** Before tagging or publishing, run `hatch run check-version-sources` (or `python scripts/check_version_sources.py`). It exits non-zero with a clear diff if `pyproject.toml`, `setup.py`, `src/__init__.py`, and `src/specfact_cli/__init__.py` disagree. The **Tests** job in `.github/workflows/pr-orchestrator.yml` runs the same script so mismatches fail CI. Pre-commit runs it whenever a version file is staged (see the `check-version-sources` hook in `.pre-commit-config.yaml`) instead of treating version-only commits as “safe” without verification. - `hatch run release` is reserved for maintainers to chain `check-version-sources` before manual release steps; extend that script if you add more release automation. - `feature/*` branches imply a minor bump, `bugfix/*` and `hotfix/*` imply a patch bump, and major bumps require explicit confirmation. -## Changelog +### Changelog - Update `CHANGELOG.md` in the same commit as the version bump. - Follow Keep a Changelog sections: `Added`, `Changed`, `Fixed`, `Removed`, `Security`. -## Commits +### Commits - Use Conventional Commits. - If signed commits fail in a non-interactive shell, stage files and hand the exact `git commit -S -m ""` command to the user instead of bypassing signing. -## Documentation and README +### Documentation and README - Keep docs current with every user-facing behavior change. - Preserve all Jekyll frontmatter on docs edits. - Update navigation when adding or moving pages. - Keep `README.md` and the docs landing page aligned with what SpecFact actually does. -## Internal wiki (sibling `specfact-cli-internal`) +### Internal wiki (sibling `specfact-cli-internal`) After **merging** changes that affect OpenSpec or GitHub-linked planning, and when a sibling `specfact-cli-internal` checkout is available, run the wiki scripts only after **`cd` into that internal repo** so the working directory matches what the scripts expect (running from `specfact-cli` or elsewhere will break them). From this repo’s root, for example: diff --git a/docs/modules/code-review.md b/docs/modules/code-review.md index c3dbffb0..ceb2ee09 100644 --- a/docs/modules/code-review.md +++ b/docs/modules/code-review.md @@ -102,26 +102,18 @@ The scaffolded `ReviewReport` envelope carries these fields: ## Pre-Commit Review Gate -This repository wires `specfact code review run` into the smart pre-commit wrapper before a commit -is considered green. - -The supported local hook entry lives in `.pre-commit-config.yaml`: - -```yaml -repos: - - repo: local - hooks: - - id: specfact-smart-checks - name: SpecFact smart pre-commit checks - entry: scripts/pre-commit-smart-checks.sh - language: script - pass_filenames: false - always_run: true -``` - -The wrapper calls `scripts/pre_commit_code_review.py` only when staged Python files are present, -alongside the repo's other local required gates (module signatures, formatter safety, Markdown/YAML -checks, workflow lint when relevant, and contract-test fast feedback). The review helper itself +This repository wires `specfact code review run` into **Block 2** of the modular pre-commit pipeline +(`scripts/pre-commit-quality-checks.sh block2`), configured in `.pre-commit-config.yaml` alongside +hooks that mirror `specfact-cli-modules` (module verify, format, staged YAML/Markdown/workflow checks, +`hatch run lint` when Python is staged, then code review + contract tests). + +Downstream copies can either use the full modular config from this repo or a single hook +`specfact-smart-checks` pointing at `scripts/pre-commit-smart-checks.sh` (shim → `pre-commit-quality-checks.sh all`). + +Block 2 calls `scripts/pre_commit_code_review.py` with staged paths under `src/`, `scripts/`, +`tools/`, `tests/`, and `openspec/changes/` (non-Python paths are filtered inside the helper), +after Block 1 gates (module signatures, formatter safety, Markdown/YAML/workflow checks, and full +`hatch run lint` when `.py` is staged). The review helper itself then runs: ```bash diff --git a/pyproject.toml b/pyproject.toml index 9430a471..75638793 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "specfact-cli" -version = "0.46.0" +version = "0.46.1" description = "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with validation and contract enforcement for new projects and long-lived codebases." readme = "README.md" requires-python = ">=3.11" @@ -285,7 +285,7 @@ contract-prune = "python tools/migrate_tests_to_contracts.py --prune --dry-run" # Pre-commit hooks pre-commit-install = "bash scripts/setup-git-hooks.sh" -pre-commit-checks = "bash scripts/pre-commit-smart-checks.sh" +pre-commit-checks = "bash scripts/pre-commit-quality-checks.sh all" pre-commit-test = "bash scripts/pre-commit-smart-test.sh" [tool.hatch.envs.py311] diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh new file mode 100755 index 00000000..d1f103b7 --- /dev/null +++ b/scripts/pre-commit-quality-checks.sh @@ -0,0 +1,449 @@ +#!/usr/bin/env bash +# Pre-commit quality checks for specfact-cli (layout parity with specfact-cli-modules). +# +# Pre-commit buffers output until each hook finishes; split into subcommands so each stage +# completes and prints before the next hook starts (see .pre-commit-config.yaml). +# +# Subcommands: block1-format | block1-yaml | block1-markdown-fix | block1-markdown-lint | +# block1-workflows | block1-lint | block2 | all +# +# Note: specfact-cli has no packages/ tree; there is no bundle-import hook (see +# specfact-cli-modules check-bundle-imports). Module signature verification is a separate +# pre-commit hook in .pre-commit-config.yaml, matching the modules repo. + +set -e + +RED='\033[0;31m' +GREEN='\033[0;32m' +YELLOW='\033[1;33m' +BLUE='\033[0;34m' +NC='\033[0m' + +info() { echo -e "${BLUE}$*${NC}" >&2; } +success() { echo -e "${GREEN}$*${NC}" >&2; } +warn() { echo -e "${YELLOW}$*${NC}" >&2; } +error() { echo -e "${RED}$*${NC}" >&2; } + +print_block1_overview() { + echo "" >&2 + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" >&2 + echo " specfact-cli pre-commit — Block 1: quality checks" >&2 + echo " format → YAML (staged) → Markdown fix/lint (staged) → workflows (staged) → lint (staged Python)" >&2 + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" >&2 + echo "" >&2 +} + +print_block2_overview() { + echo "" >&2 + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" >&2 + echo " specfact-cli pre-commit — Block 2: code review + contract tests" >&2 + echo " 1/2 code review gate (staged Python under src/, scripts/, tools/, tests/, openspec/changes/)" >&2 + echo " 2/2 contract-first tests (contract-test-status → hatch run contract-test)" >&2 + echo "━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━" >&2 + echo "" >&2 +} + +staged_files() { + git diff --cached --name-only +} + +has_staged_yaml() { + staged_files | grep -E '\.ya?ml$' >/dev/null 2>&1 +} + +has_staged_workflows() { + staged_files | grep -E '^\.github/workflows/.*\.ya?ml$' >/dev/null 2>&1 +} + +has_staged_markdown() { + staged_files | grep -E '\.md$' >/dev/null 2>&1 +} + +has_staged_python() { + staged_files | grep -E '\.pyi?$' >/dev/null 2>&1 +} + +staged_markdown_files() { + staged_files | grep -E '\.md$' || true +} + +# Paths eligible for the code review gate (parity with modules: scoped prefixes; non-Python filtered by pre_commit_code_review.py). +staged_review_gate_files() { + local line + while IFS= read -r line; do + [ -z "${line}" ] && continue + case "${line}" in + */TDD_EVIDENCE.md|TDD_EVIDENCE.md) continue ;; + src/*|scripts/*|tools/*|tests/*|openspec/changes/*) + printf '%s\n' "${line}" + ;; + esac + done < <(staged_files) +} + +fail_if_markdown_has_unstaged_hunks() { + local md_files + md_files=$(staged_markdown_files) + if [ -z "${md_files}" ]; then + return + fi + local file + while IFS= read -r file; do + [ -z "${file}" ] && continue + if ! git diff --quiet -- "$file"; then + error "❌ Cannot auto-fix Markdown with unstaged hunks: $file" + warn "💡 Stage the full file or stash/revert the unstaged Markdown changes before commit" + exit 1 + fi + done </dev/null 2>&1; then + if echo "${md_files}" | xargs -r markdownlint --fix --config .markdownlint.json; then + echo "${md_files}" | xargs -r git add -- + success "✅ Block 1 — Markdown auto-fix applied" + else + error "❌ Block 1 — Markdown auto-fix failed" + exit 1 + fi + else + if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --fix --config .markdownlint.json; then + echo "${md_files}" | xargs -r git add -- + success "✅ Block 1 — Markdown auto-fix applied (npx)" + else + error "❌ Block 1 — Markdown auto-fix failed (npx)" + warn "💡 Install markdownlint-cli globally for faster hooks: npm i -g markdownlint-cli" + exit 1 + fi + fi +} + +run_markdown_lint_if_needed() { + if ! has_staged_markdown; then + info "📦 Block 1 — Markdown lint — skipped (no staged *.md)" + return + fi + info "📦 Block 1 — Markdown lint — running markdownlint" + local md_files + md_files=$(staged_markdown_files) + if [ -z "${md_files}" ]; then + return + fi + if command -v markdownlint >/dev/null 2>&1; then + if echo "${md_files}" | xargs -r markdownlint --config .markdownlint.json; then + success "✅ Block 1 — Markdown lint passed" + else + error "❌ Block 1 — Markdown lint failed" + exit 1 + fi + else + if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --config .markdownlint.json; then + success "✅ Block 1 — Markdown lint passed (npx)" + else + error "❌ Block 1 — Markdown lint failed (npx)" + exit 1 + fi + fi +} + +run_workflow_lint_if_needed() { + if has_staged_workflows; then + info "📦 Block 1 — workflows — running \`hatch run lint-workflows\`" + if hatch run lint-workflows; then + success "✅ Block 1 — workflow lint passed" + else + error "❌ Block 1 — workflow lint failed" + exit 1 + fi + else + info "📦 Block 1 — workflows — skipped (no staged .github/workflows/*.yml)" + fi +} + +run_lint_if_staged_python() { + if ! has_staged_python; then + info "📦 Block 1 — lint — skipped (no staged *.py / *.pyi)" + return 0 + fi + info "📦 Block 1 — lint — running \`hatch run lint\` (ruff, basedpyright, pylint; matches CI quality gate)" + if hatch run lint; then + success "✅ Block 1 — lint passed" + else + error "❌ Block 1 — lint failed" + warn "💡 Run: hatch run lint" + exit 1 + fi +} + +run_code_review_gate() { + local review_array=() + while IFS= read -r line; do + [ -z "${line}" ] && continue + review_array+=("${line}") + done < <(staged_review_gate_files) + + if [ ${#review_array[@]} -eq 0 ]; then + info "📦 Block 2 — code review — skipped (no staged paths under src/, scripts/, tools/, tests/, or openspec/changes/)" + return + fi + + info "📦 Block 2 — code review — running \`hatch run python scripts/pre_commit_code_review.py\` (${#review_array[@]} path(s))" + if hatch run python scripts/pre_commit_code_review.py "${review_array[@]}"; then + success "✅ Block 2 — code review gate passed" + else + error "❌ Block 2 — code review gate failed" + warn "💡 Fix blocking review findings or run: hatch run python scripts/pre_commit_code_review.py " + exit 1 + fi +} + +run_contract_tests_visible() { + info "📦 Block 2 — contract tests — running \`hatch run contract-test-status\`" + if hatch run contract-test-status >/dev/null 2>&1; then + success "✅ Block 2 — contract tests — skipped (contract-test-status: no input changes)" + else + info "📦 Block 2 — contract tests — running \`hatch run contract-test\`" + if hatch run contract-test; then + success "✅ Block 2 — contract-first tests passed" + warn "💡 CI may still run the full quality matrix" + else + error "❌ Block 2 — contract-first tests failed" + warn "💡 Run: hatch run contract-test-status" + exit 1 + fi + fi +} + +run_block1_format() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: format" + print_block1_overview + run_format_safety +} + +run_block1_yaml() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: YAML" + run_yaml_lint_if_needed +} + +run_block1_markdown_fix() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: Markdown auto-fix" + run_markdown_autofix_if_needed +} + +run_block1_markdown_lint() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: Markdown lint" + run_markdown_lint_if_needed +} + +run_block1_workflows() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: workflow lint" + run_workflow_lint_if_needed +} + +run_block1_lint() { + warn "🔍 specfact-cli pre-commit — Block 1 — hook: lint" + run_lint_if_staged_python +} + +run_block2() { + warn "🔍 specfact-cli pre-commit — Block 2 — hook: review + contract tests" + if check_safe_change; then + success "✅ Safe change detected — skipping Block 2 (code review + contract tests)" + info "💡 Only docs, workflow, version metadata, or allowlisted infra changed" + exit 0 + fi + print_block2_overview + run_code_review_gate + if [ ! -f "tools/contract_first_smart_test.py" ]; then + error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" + exit 1 + fi + run_contract_tests_visible +} + +run_all() { + warn "🔍 Running full specfact-cli pre-commit pipeline (\`all\` — manual or CI)" + print_block1_overview + run_module_signature_verification + run_version_sources_check_if_needed + run_format_safety + run_yaml_lint_if_needed + run_markdown_autofix_if_needed + run_markdown_lint_if_needed + run_workflow_lint_if_needed + run_lint_if_staged_python + success "✅ Block 1 complete (all stages passed or skipped as expected)" + if check_safe_change; then + success "✅ Safe change detected — skipping Block 2 (code review + contract tests)" + exit 0 + fi + print_block2_overview + run_code_review_gate + if [ ! -f "tools/contract_first_smart_test.py" ]; then + error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" + exit 1 + fi + run_contract_tests_visible +} + +usage_error() { + error "Usage: $0 {block1-format|block1-yaml|block1-markdown-fix|block1-markdown-lint|block1-workflows|block1-lint|block2|all} (also: -h | --help | help)" + exit 2 +} + +show_help() { + echo "Usage: $0 {block1-format|block1-yaml|block1-markdown-fix|block1-markdown-lint|block1-workflows|block1-lint|block2|all}" >&2 + echo "Help aliases: -h, --help, help" >&2 + exit 0 +} + +main() { + case "${1:-all}" in + block1-format) + run_block1_format + ;; + block1-yaml) + run_block1_yaml + ;; + block1-markdown-fix) + run_block1_markdown_fix + ;; + block1-markdown-lint) + run_block1_markdown_lint + ;; + block1-workflows) + run_block1_workflows + ;; + block1-lint) + run_block1_lint + ;; + block2) + run_block2 + ;; + all) + run_all + ;; + -h|--help|help) + show_help + ;; + *) + usage_error + ;; + esac +} + +main "$@" diff --git a/scripts/pre-commit-smart-checks.sh b/scripts/pre-commit-smart-checks.sh index f7a01e5a..3cc69737 100755 --- a/scripts/pre-commit-smart-checks.sh +++ b/scripts/pre-commit-smart-checks.sh @@ -1,334 +1,5 @@ #!/usr/bin/env bash -# Pre-commit checks: YAML lint, GitHub workflow lint, and contract-first smart tests. -# - Always runs YAML/workflow lint when relevant files are staged. -# - Skips tests for safe-only changes (version/docs/test infra), but still enforces YAML/workflow lint. - -set -e - -# Colors for output -RED='\033[0;31m' -GREEN='\033[0;32m' -YELLOW='\033[1;33m' -BLUE='\033[0;34m' -NC='\033[0m' # No Color - -info() { echo -e "${BLUE}$*${NC}"; } -success(){ echo -e "${GREEN}$*${NC}"; } -warn() { echo -e "${YELLOW}$*${NC}"; } -error() { echo -e "${RED}$*${NC}"; } - -staged_files() { - git diff --cached --name-only -} - -has_staged_yaml() { - staged_files | grep -E '\\.ya?ml$' >/dev/null 2>&1 -} - -has_staged_workflows() { - staged_files | grep -E '^\.github/workflows/.*\\.ya?ml$' >/dev/null 2>&1 -} - -has_staged_markdown() { - staged_files | grep -E '\\.md$' >/dev/null 2>&1 -} - -staged_python_files() { - staged_files | grep -E '\\.pyi?$' || true -} - -staged_markdown_files() { - staged_files | grep -E '\\.md$' || true -} - -fail_if_markdown_has_unstaged_hunks() { - local md_files - md_files=$(staged_markdown_files) - if [ -z "${md_files}" ]; then - return - fi - - local file - while IFS= read -r file; do - [ -z "${file}" ] && continue - if ! git diff --quiet -- "$file"; then - error "❌ Cannot auto-fix Markdown with unstaged hunks: $file" - warn "💡 Stage the full file or stash/revert the unstaged Markdown changes before commit" - exit 1 - fi - done </dev/null 2>&1; then - if echo "${md_files}" | xargs -r markdownlint --fix --config .markdownlint.json; then - echo "${md_files}" | xargs -r git add -- - success "✅ Markdown auto-fix applied" - else - error "❌ Markdown auto-fix failed" - exit 1 - fi - else - if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --fix --config .markdownlint.json; then - echo "${md_files}" | xargs -r git add -- - success "✅ Markdown auto-fix applied (npx)" - else - error "❌ Markdown auto-fix failed (npx)" - warn "💡 Install markdownlint-cli globally for faster hooks: npm i -g markdownlint-cli" - exit 1 - fi - fi - else - info "ℹ️ No staged Markdown changes — skipping markdown auto-fix" - fi -} - -run_markdown_lint_if_needed() { - if has_staged_markdown; then - info "📝 Markdown changes detected — running markdownlint" - local md_files - md_files=$(staged_markdown_files) - if [ -z "${md_files}" ]; then - info "ℹ️ No staged markdown files resolved — skipping markdownlint" - return - fi - - if command -v markdownlint >/dev/null 2>&1; then - if echo "${md_files}" | xargs -r markdownlint --config .markdownlint.json; then - success "✅ Markdown lint passed" - else - error "❌ Markdown lint failed" - exit 1 - fi - else - if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --config .markdownlint.json; then - success "✅ Markdown lint passed (npx)" - else - error "❌ Markdown lint failed (npx)" - warn "💡 Install markdownlint-cli globally for faster hooks: npm i -g markdownlint-cli" - exit 1 - fi - fi - else - info "ℹ️ No staged Markdown changes — skipping markdownlint" - fi -} - -run_format_safety() { - info "🧹 Running formatter safety check (hatch run format)" - local before_unstaged after_unstaged - before_unstaged=$(git diff --binary -- . || true) - if hatch run format; then - after_unstaged=$(git diff --binary -- . || true) - if [ "${before_unstaged}" != "${after_unstaged}" ]; then - error "❌ Formatter changed files. Review and re-stage before committing." - warn "💡 Run: hatch run format && git add -A" - exit 1 - fi - success "✅ Formatting check passed" - else - error "❌ Formatting check failed" - exit 1 - fi -} - -run_yaml_lint_if_needed() { - if has_staged_yaml; then - info "🔎 YAML changes detected — running yamllint (relaxed)" - if hatch run yaml-lint; then - success "✅ YAML lint passed" - else - error "❌ YAML lint failed" - exit 1 - fi - else - info "ℹ️ No staged YAML changes — skipping yamllint" - fi -} - -run_actionlint_if_needed() { - if has_staged_workflows; then - info "🔎 GitHub workflow changes detected — running actionlint" - if hatch run lint-workflows; then - success "✅ Workflow lint passed" - else - error "❌ Workflow lint failed" - exit 1 - fi - else - info "ℹ️ No staged workflow YAML changes — skipping actionlint" - fi -} - -run_code_review_gate() { - # Build a bash array so we invoke pre_commit_code_review.py exactly once. Using xargs - # here can split into multiple subprocesses when the argument list is long (default - # max-chars), each overwriting .specfact/code-review.json — yielding partial or empty - # findings and a misleading artifact. - local py_array=() - while IFS= read -r line; do - [ -z "${line}" ] && continue - py_array+=("${line}") - done < <(staged_python_files) - - if [ ${#py_array[@]} -eq 0 ]; then - info "ℹ️ No staged Python files — skipping code review gate" - return - fi - - info "🛡️ Running code review gate on staged Python files" - if hatch run python scripts/pre_commit_code_review.py "${py_array[@]}"; then - success "✅ Code review gate passed" - else - error "❌ Code review gate failed" - warn "💡 Fix blocking review findings or run the gate manually with: hatch run python scripts/pre_commit_code_review.py " - exit 1 - fi -} - -check_safe_change() { - local files - files=$(staged_files) - local version_files=("pyproject.toml" "setup.py" "src/__init__.py" "src/specfact_cli/__init__.py") - local changelog_files=("CHANGELOG.md") - local test_infrastructure_files=( - "tools/smart_test_coverage.py" - "scripts/pre-commit-smart-checks.sh" - "tools/functional_coverage_analyzer.py" - ) - local doc_patterns=("*.md" "*.rst" "*.txt" "*.json" "*.yaml" "*.yml") - local doc_dirs=("docs/" "papers/" "presentations/" "images/") - - local version_changes=0 - local test_infra_changes=0 - local doc_changes=0 - local other_changes=0 - - for file in $files; do - local is_safe=false - - if [[ " ${version_files[@]} " =~ " ${file} " ]]; then - version_changes=$((version_changes + 1)) - is_safe=true - elif [[ " ${changelog_files[@]} " =~ " ${file} " ]]; then - doc_changes=$((doc_changes + 1)) - is_safe=true - elif [[ " ${test_infrastructure_files[@]} " =~ " ${file} " ]]; then - test_infra_changes=$((test_infra_changes + 1)) - is_safe=true - elif [[ "$file" == *.md || "$file" == *.rst || "$file" == *.txt || "$file" == *.json || "$file" == *.yaml || "$file" == *.yml ]]; then - doc_changes=$((doc_changes + 1)) - is_safe=true - elif [[ "$file" == docs/* || "$file" == papers/* || "$file" == presentations/* || "$file" == images/* ]]; then - doc_changes=$((doc_changes + 1)) - is_safe=true - fi - - if [ "$is_safe" = false ]; then - other_changes=$((other_changes + 1)) - fi - done - - if [ $other_changes -eq 0 ] && [ $((version_changes + test_infra_changes + doc_changes)) -gt 0 ]; then - return 0 - fi - return 1 -} - -warn "🔍 Running pre-commit checks (YAML/workflows + smart tests)" - -# Always enforce module signature/version policy before commit -run_module_signature_verification -run_version_sources_check_if_needed -run_format_safety - -# Always run lint checks when relevant files changed -run_markdown_autofix_if_needed -run_markdown_lint_if_needed -run_yaml_lint_if_needed -run_actionlint_if_needed - -# If only safe changes, skip tests after lint passes (version files already verified above) -if check_safe_change; then - success "✅ Safe change detected - skipping test run" - info "💡 Only version numbers, docs/test infra, or YAML/workflows changed" - exit 0 -fi - -run_code_review_gate - -# Contract-first test flow -if [ ! -f "tools/contract_first_smart_test.py" ]; then - error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" - exit 1 -fi - -if hatch run contract-test-status > /dev/null 2>&1; then - success "✅ No changes detected - using cached contract test data" - exit 0 -else - warn "🔄 Changes detected - running contract-first tests for fast feedback..." - if hatch run contract-test; then - success "✅ Contract-first tests passed - ready to commit" - warn "💡 GitHub Actions will run full contract test suite" - exit 0 - else - error "❌ Contract-first tests failed" - warn "💡 Run 'hatch run contract-test-status' for details" - warn "💡 Or run 'hatch run contract-test-full' for full test suite" - warn "💡 Legacy: 'hatch run smart-test-force' for smart test suite" - exit 1 - fi -fi +# Back-compat entry: single hook for downstream repos that pin `specfact-smart-checks`. +# Canonical layout is modular hooks in .pre-commit-config.yaml → pre-commit-quality-checks.sh. +set -euo pipefail +exec "$(dirname "$0")/pre-commit-quality-checks.sh" all "$@" diff --git a/scripts/setup-git-hooks.sh b/scripts/setup-git-hooks.sh index ea85dfef..e53d9a77 100755 --- a/scripts/setup-git-hooks.sh +++ b/scripts/setup-git-hooks.sh @@ -45,6 +45,9 @@ fi echo -e "${GREEN}🎉 Git hooks setup complete!${NC}" echo "" +echo "Prefer \`pre-commit install\` for the modular hook layout (see .pre-commit-config.yaml);" +echo "the copied script runs the full \`pre-commit-quality-checks.sh all\` pipeline as a fallback." +echo "" echo "The pre-commit hook will now:" echo " • Verify module signatures and enforce version bumps" echo " • Run hatch formatter safety check and fail if files are changed" diff --git a/setup.py b/setup.py index 28d012b7..92957309 100644 --- a/setup.py +++ b/setup.py @@ -7,7 +7,7 @@ if __name__ == "__main__": _setup = setup( name="specfact-cli", - version="0.46.0", + version="0.46.1", description=( "The swiss knife CLI for agile DevOps teams. Keep backlog, specs, tests, and code in sync with " "validation and contract enforcement for new projects and long-lived codebases." diff --git a/src/__init__.py b/src/__init__.py index ba6c4e34..e02e583c 100644 --- a/src/__init__.py +++ b/src/__init__.py @@ -3,4 +3,4 @@ """ # Package version: keep in sync with pyproject.toml, setup.py, src/specfact_cli/__init__.py -__version__ = "0.46.0" +__version__ = "0.46.1" diff --git a/src/specfact_cli/__init__.py b/src/specfact_cli/__init__.py index db76e90c..e22a7f8b 100644 --- a/src/specfact_cli/__init__.py +++ b/src/specfact_cli/__init__.py @@ -45,6 +45,6 @@ def _bootstrap_bundle_paths() -> None: _bootstrap_bundle_paths() -__version__ = "0.46.0" +__version__ = "0.46.1" __all__ = ["__version__"] diff --git a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py index 5688de23..d6fa4185 100644 --- a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py +++ b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py @@ -3,33 +3,50 @@ from pathlib import Path -def _script_text() -> str: +def _quality_script_text() -> str: + return (Path(__file__).resolve().parents[3] / "scripts" / "pre-commit-quality-checks.sh").read_text( + encoding="utf-8" + ) + + +def _smart_shim_text() -> str: return (Path(__file__).resolve().parents[3] / "scripts" / "pre-commit-smart-checks.sh").read_text(encoding="utf-8") def test_pre_commit_markdown_checks_run_autofix_before_lint() -> None: - script = _script_text() + script = _quality_script_text() assert "run_markdown_autofix_if_needed" in script assert "markdownlint --fix --config .markdownlint.json" in script - assert "run_markdown_autofix_if_needed\nrun_markdown_lint_if_needed" in script + idx_fix = script.find("run_markdown_autofix_if_needed") + idx_lint = script.find("run_markdown_lint_if_needed") + assert 0 <= idx_fix < idx_lint, "auto-fix must be defined before lint in the script" def test_pre_commit_markdown_autofix_restages_files() -> None: - script = _script_text() + script = _quality_script_text() assert "xargs -r git add --" in script def test_pre_commit_markdown_autofix_rejects_partial_staging() -> None: - script = _script_text() + script = _quality_script_text() assert 'git diff --quiet -- "$file"' in script assert "Cannot auto-fix Markdown with unstaged hunks" in script def test_pre_commit_runs_code_review_gate_before_contract_tests() -> None: - script = _script_text() + script = _quality_script_text() assert "run_code_review_gate" in script assert "hatch run python scripts/pre_commit_code_review.py" in script - assert "run_code_review_gate\n\n# Contract-first test flow" in script - # Single invocation with all staged files — xargs can split into multiple runs and - # clobber .specfact/code-review.json (partial or empty findings). - assert '"${py_array[@]}"' in script + block2 = script.find("run_block2()") + assert block2 != -1 + tail = script[block2:] + idx_gate = tail.find("run_code_review_gate") + idx_contract = tail.find("run_contract_tests_visible") + assert 0 <= idx_gate < idx_contract + assert '"${review_array[@]}"' in script + + +def test_pre_commit_smart_checks_shim_delegates_to_quality_all() -> None: + shim = _smart_shim_text() + assert "pre-commit-quality-checks.sh" in shim + assert 'all "$@"' in shim diff --git a/tests/unit/workflows/test_trustworthy_green_checks.py b/tests/unit/workflows/test_trustworthy_green_checks.py index 1d44eeec..23ce194b 100644 --- a/tests/unit/workflows/test_trustworthy_green_checks.py +++ b/tests/unit/workflows/test_trustworthy_green_checks.py @@ -165,15 +165,36 @@ def test_module_signature_check_name_is_canonical_across_workflows() -> None: assert orchestrator_name == dedicated_name == "Verify Module Signatures" -def test_pre_commit_config_installs_supported_smart_check_wrapper() -> None: - """The supported local hook path should expose the same gate semantics as CI.""" +def test_pre_commit_config_matches_modular_quality_layout() -> None: + """Local hooks should mirror specfact-cli-modules: fail_fast, verify, block1 stages, block2.""" + config = _load_yaml(PRE_COMMIT_CONFIG) + assert config.get("fail_fast") is True hooks = _load_hooks() - matching = [hook for hook in hooks if hook.get("entry") == "scripts/pre-commit-smart-checks.sh"] - assert matching, "Expected .pre-commit-config.yaml to expose the smart-check wrapper hook" - hook = matching[0] - assert hook.get("id") == "specfact-smart-checks", "Hook id must remain stable for pre-commit consumers" - assert hook.get("pass_filenames") is False - assert hook.get("language") == "script" + by_id = {h.get("id"): h for h in hooks if isinstance(h.get("id"), str)} + assert "verify-module-signatures" in by_id + assert by_id["verify-module-signatures"].get("always_run") is True + assert "--payload-from-filesystem" in str(by_id["verify-module-signatures"].get("entry", "")) + assert "check-version-sources" in by_id + assert "cli-block1-format" in by_id + assert "cli-block1-yaml" in by_id + assert "cli-block1-markdown-fix" in by_id + assert "cli-block1-markdown-lint" in by_id + assert "cli-block1-workflows" in by_id + assert "cli-block1-lint" in by_id + assert "cli-block2" in by_id + assert by_id["cli-block2"].get("always_run") is True + for hid in ( + "cli-block1-format", + "cli-block1-yaml", + "cli-block1-markdown-fix", + "cli-block1-markdown-lint", + "cli-block1-workflows", + "cli-block1-lint", + "cli-block2", + ): + entry = by_id[hid].get("entry", "") + assert "pre-commit-quality-checks.sh" in str(entry), f"{hid} must invoke quality-checks script" + assert "check-doc-frontmatter" in by_id def test_coderabbit_auto_review_covers_dev_and_main() -> None: From 8c497c668bd059dbd2945d23f7676a90b9c9fb8e Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 18:46:26 +0200 Subject: [PATCH 2/8] fix(pre-commit): branch-aware module verify hook (marketplace-06 policy) - Add scripts/pre-commit-verify-modules.sh and git-branch-module-signature-flag.sh - Point verify-module-signatures hook at wrapper (script); skip when no staged module paths - pre-commit-quality-checks all: delegate module step to wrapper; safe-change allowlist - Tests + CONTRIBUTING/CHANGELOG alignment Made-with: Cursor --- .pre-commit-config.yaml | 6 +- CHANGELOG.md | 4 ++ CONTRIBUTING.md | 5 +- scripts/git-branch-module-signature-flag.sh | 15 ++++ scripts/pre-commit-quality-checks.sh | 15 ++-- scripts/pre-commit-verify-modules.sh | 26 +++++++ .../scripts/test_pre_commit_verify_modules.py | 71 +++++++++++++++++++ .../test_trustworthy_green_checks.py | 9 ++- 8 files changed, 139 insertions(+), 12 deletions(-) create mode 100755 scripts/git-branch-module-signature-flag.sh create mode 100755 scripts/pre-commit-verify-modules.sh create mode 100644 tests/unit/scripts/test_pre_commit_verify_modules.py diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0d94c173..4ccb3538 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -6,9 +6,9 @@ repos: - repo: local hooks: - id: verify-module-signatures - name: Verify module signatures and version bumps - entry: hatch run ./scripts/verify-modules-signature.py --require-signature --payload-from-filesystem --enforce-version-bump - language: system + name: Verify module signatures (branch-aware; skip if no staged module tree changes) + entry: scripts/pre-commit-verify-modules.sh + language: script pass_filenames: false always_run: true diff --git a/CHANGELOG.md b/CHANGELOG.md index 3032ee08..66aea3f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,10 @@ All notable changes to this project will be documented in this file. Python is staged, then Block 2 (scoped code review + contract tests). New `scripts/pre-commit-quality-checks.sh`; `scripts/pre-commit-smart-checks.sh` remains a shim for downstream `specfact-smart-checks` consumers. +- **Module verify (pre-commit)**: branch-aware policy via `scripts/pre-commit-verify-modules.sh` and + `scripts/git-branch-module-signature-flag.sh` — `--allow-unsigned` off `main`, `--require-signature` on + `main`; skips when no staged paths under `modules/` or `src/specfact_cli/modules/`; always uses + `--payload-from-filesystem` with `--enforce-version-bump` when the check runs. --- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 8639464f..22817115 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,7 +79,8 @@ hatch test --cover -v ### Pre-commit Checks Local hooks use **`fail_fast: true`** and a **modular layout** aligned with `specfact-cli-modules`: -verify module signatures → sync version files when those paths are staged → format (always) → +branch-aware module verify (skip if no staged module tree changes; `--allow-unsigned` except on + `main`, where `--require-signature`) → sync version files when those paths are staged → format (always) → YAML / Markdown / workflow lint when matching paths are staged → **`hatch run lint`** when Python is staged → Block 2 (scoped code review + contract tests, with a safe-change short-circuit for docs-only and similar commits). See `.pre-commit-config.yaml` and `scripts/pre-commit-quality-checks.sh`. @@ -110,7 +111,7 @@ hatch run contract-test-full The supported local hook path is the repo-owned smart-check wrapper installed by the commands above. It keeps local semantics aligned with CI: -- Merge-blocking local gates: module signature verification, formatter safety, Markdown/YAML checks, +- Merge-blocking local gates: module signature verification (branch-aware; see `scripts/pre-commit-verify-modules.sh`), formatter safety, Markdown/YAML checks, workflow lint for staged workflow changes, and contract-test fast feedback when code changes. - Review gate behavior: `specfact code review run` reviews staged Python files and blocks the commit only on `FAIL`. `PASS_WITH_ADVISORY` remains green but still prints the JSON report path for diff --git a/scripts/git-branch-module-signature-flag.sh b/scripts/git-branch-module-signature-flag.sh new file mode 100755 index 00000000..02d97904 --- /dev/null +++ b/scripts/git-branch-module-signature-flag.sh @@ -0,0 +1,15 @@ +#!/usr/bin/env bash +# Emit verify-modules-signature.py signature policy flag for the current git branch. +# Prints --require-signature on main; --allow-unsigned elsewhere (including detached HEAD). +set -euo pipefail + +branch="" +branch=$(git branch --show-current 2>/dev/null || true) +if [[ -z "${branch}" || "${branch}" == "HEAD" ]]; then + branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || true) +fi +if [[ "${branch}" == "main" ]]; then + printf '%s\n' "--require-signature" +else + printf '%s\n' "--allow-unsigned" +fi diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index d1f103b7..c8724130 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -114,7 +114,7 @@ check_safe_change() { case "${file}" in pyproject.toml|setup.py|src/__init__.py|src/specfact_cli/__init__.py) ;; CHANGELOG.md|README.md|.pre-commit-config.yaml) ;; - scripts/pre-commit-quality-checks.sh|scripts/pre-commit-smart-checks.sh) ;; + scripts/pre-commit-quality-checks.sh|scripts/pre-commit-smart-checks.sh|scripts/pre-commit-verify-modules.sh|scripts/git-branch-module-signature-flag.sh) ;; tools/smart_test_coverage.py|tools/functional_coverage_analyzer.py) ;; *.md|*.rst|*.txt|*.json|*.yaml|*.yml) ;; docs/*|papers/*|presentations/*|images/*) ;; @@ -157,12 +157,17 @@ run_version_sources_check_if_needed() { } run_module_signature_verification() { - info "🔐 Verifying bundled module signatures/version bumps (strict)" - if hatch run ./scripts/verify-modules-signature.py --require-signature --payload-from-filesystem --enforce-version-bump; then - success "✅ Module signature/version verification passed" + local root + root=$(git rev-parse --show-toplevel 2>/dev/null || true) + if [ -z "${root}" ]; then + error "❌ Cannot resolve git repository root for module signature verification" + exit 1 + fi + if bash "${root}/scripts/pre-commit-verify-modules.sh"; then + success "✅ Module signature/version verification passed (or skipped — no staged module tree changes)" else error "❌ Module signature/version verification failed" - warn "💡 Re-sign changed modules with version bump before commit" + warn "💡 On main use --require-signature; elsewhere CI signs after PR approval" exit 1 fi } diff --git a/scripts/pre-commit-verify-modules.sh b/scripts/pre-commit-verify-modules.sh new file mode 100755 index 00000000..a30d705d --- /dev/null +++ b/scripts/pre-commit-verify-modules.sh @@ -0,0 +1,26 @@ +#!/usr/bin/env bash +# Pre-commit / manual entry: branch-aware module verify (marketplace-06 policy). +# Skips when nothing under modules/ or src/specfact_cli/modules/ is staged. +set -euo pipefail + +repo_root=$(git rev-parse --show-toplevel 2>/dev/null || true) +if [[ -z "${repo_root}" ]]; then + echo "❌ Cannot resolve git repository root for module signature verification" >&2 + exit 1 +fi +cd "${repo_root}" + +staged_files=$(git diff --cached --name-only 2>/dev/null || true) +if ! echo "${staged_files}" | grep -qE '^(src/specfact_cli/modules|modules)/'; then + echo "ℹ️ No staged changes under modules/ or src/specfact_cli/modules/ — skipping module signature verification" + exit 0 +fi + +flag_script="${repo_root}/scripts/git-branch-module-signature-flag.sh" +if [[ ! -f "${flag_script}" ]]; then + echo "❌ Missing ${flag_script}" >&2 + exit 1 +fi +sig_flag=$(bash "${flag_script}") +echo "🔐 Verifying bundled module manifests (${sig_flag}, --enforce-version-bump, --payload-from-filesystem)" >&2 +exec hatch run ./scripts/verify-modules-signature.py "${sig_flag}" --enforce-version-bump --payload-from-filesystem diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py new file mode 100644 index 00000000..26ef94a2 --- /dev/null +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -0,0 +1,71 @@ +"""Branch-aware module verify wrapper used by pre-commit (marketplace-06 policy).""" + +from __future__ import annotations + +import subprocess +from pathlib import Path + +import pytest + + +REPO_ROOT = Path(__file__).resolve().parents[3] +FLAG_SCRIPT = REPO_ROOT / "scripts" / "git-branch-module-signature-flag.sh" +VERIFY_WRAPPER = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" + + +def test_verify_wrapper_invokes_branch_flag_and_payload_from_filesystem() -> None: + body = VERIFY_WRAPPER.read_text(encoding="utf-8") + assert "git-branch-module-signature-flag.sh" in body + assert "--payload-from-filesystem" in body + assert "--enforce-version-bump" in body + assert "verify-modules-signature.py" in body + + +def _run_flag(*, cwd: Path) -> str: + result = subprocess.run( + ["bash", str(FLAG_SCRIPT)], + cwd=cwd, + capture_output=True, + text=True, + check=False, + timeout=30, + ) + assert result.returncode == 0, result.stderr + return result.stdout.strip() + + +def _git_init_with_commit(repo: Path) -> None: + subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True, text=True) + subprocess.run( + ["git", "config", "user.email", "test@example.com"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + subprocess.run( + ["git", "config", "user.name", "Test User"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + (repo / "README.md").write_text("x\n", encoding="utf-8") + subprocess.run(["git", "add", "README.md"], cwd=repo, check=True, capture_output=True, text=True) + subprocess.run(["git", "commit", "-m", "init"], cwd=repo, check=True, capture_output=True, text=True) + + +@pytest.mark.parametrize( + ("branch", "expected"), + ( + ("feature/foo", "--allow-unsigned"), + ("dev", "--allow-unsigned"), + ("main", "--require-signature"), + ), +) +def test_git_branch_signature_flag(tmp_path: Path, branch: str, expected: str) -> None: + repo = tmp_path / "repo" + repo.mkdir() + _git_init_with_commit(repo) + subprocess.run(["git", "branch", "-M", branch], cwd=repo, check=True, capture_output=True, text=True) + assert _run_flag(cwd=repo) == expected diff --git a/tests/unit/workflows/test_trustworthy_green_checks.py b/tests/unit/workflows/test_trustworthy_green_checks.py index 23ce194b..15f7b29f 100644 --- a/tests/unit/workflows/test_trustworthy_green_checks.py +++ b/tests/unit/workflows/test_trustworthy_green_checks.py @@ -172,8 +172,13 @@ def test_pre_commit_config_matches_modular_quality_layout() -> None: hooks = _load_hooks() by_id = {h.get("id"): h for h in hooks if isinstance(h.get("id"), str)} assert "verify-module-signatures" in by_id - assert by_id["verify-module-signatures"].get("always_run") is True - assert "--payload-from-filesystem" in str(by_id["verify-module-signatures"].get("entry", "")) + verify_hook = by_id["verify-module-signatures"] + assert verify_hook.get("always_run") is True + assert verify_hook.get("language") == "script" + assert "pre-commit-verify-modules.sh" in str(verify_hook.get("entry", "")) + verify_script = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" + assert verify_script.is_file() + assert "--payload-from-filesystem" in verify_script.read_text(encoding="utf-8") assert "check-version-sources" in by_id assert "cli-block1-format" in by_id assert "cli-block1-yaml" in by_id From 689ed7bb23643ed95d5ce332b47e54d3792ac8e1 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 19:21:05 +0200 Subject: [PATCH 3/8] =?UTF-8?q?fix(pre-commit):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20portable=20quality=20checks=20and=20signature=20pol?= =?UTF-8?q?icy?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Emit require/omit from git-branch-module-signature-flag; pass --require-signature only on main - Resolve repo root in pre-commit-smart-checks via git rev-parse for .git/hooks copies - Harden pre-commit-quality-checks: ACMR staged paths, pipefail, no xargs -r, safe loops - CHANGELOG/CONTRIBUTING: Added vs Changed; document verifier CLI (no --allow-unsigned) - Tests: omit/require expectations, detached HEAD; shim asserts repo-root exec Made-with: Cursor --- CHANGELOG.md | 25 ++-- CONTRIBUTING.md | 4 +- scripts/git-branch-module-signature-flag.sh | 9 +- scripts/pre-commit-quality-checks.sh | 130 +++++++++++------- scripts/pre-commit-smart-checks.sh | 14 +- scripts/pre-commit-verify-modules.sh | 12 +- .../test_pre_commit_smart_checks_docs.py | 6 +- .../scripts/test_pre_commit_verify_modules.py | 23 +++- 8 files changed, 147 insertions(+), 76 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66aea3f7..97ef7cee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,17 +12,26 @@ All notable changes to this project will be documented in this file. ## [0.46.1] - 2026-04-14 +### Added + +- **`scripts/pre-commit-quality-checks.sh`**: modular Block 1/2 entrypoints (`block1-*`, `block2`, `all`) + with staged-file gates and Markdown auto-fix before lint (parity with `specfact-cli-modules` hook layout + and `fail_fast` behavior in `.pre-commit-config.yaml`). +- **`scripts/pre-commit-smart-checks.sh`**: back-compat shim that resolves the repository root (so copies + under `.git/hooks/pre-commit` still run the canonical quality script) and delegates to + `pre-commit-quality-checks.sh all`. + ### Changed -- **Pre-commit**: modular hook layout with `fail_fast` (parity with `specfact-cli-modules`): dedicated - module verify + version-source hook, staged YAML/Markdown/workflow gates, `hatch run lint` when - Python is staged, then Block 2 (scoped code review + contract tests). New - `scripts/pre-commit-quality-checks.sh`; `scripts/pre-commit-smart-checks.sh` remains a shim for - downstream `specfact-smart-checks` consumers. - **Module verify (pre-commit)**: branch-aware policy via `scripts/pre-commit-verify-modules.sh` and - `scripts/git-branch-module-signature-flag.sh` — `--allow-unsigned` off `main`, `--require-signature` on - `main`; skips when no staged paths under `modules/` or `src/specfact_cli/modules/`; always uses - `--payload-from-filesystem` with `--enforce-version-bump` when the check runs. + `scripts/git-branch-module-signature-flag.sh` — on `main`, run `verify-modules-signature.py` with + `--require-signature`; on other branches (including detached `HEAD`), omit that flag so the verifier + stays in checksum-only mode (there is no `--allow-unsigned` CLI). Skips when no staged paths under + `modules/` or `src/specfact_cli/modules/`; when the check runs it always passes `--payload-from-filesystem` + and `--enforce-version-bump`. +- **`scripts/pre-commit-quality-checks.sh`**: staged file enumeration uses `git diff --cached --diff-filter=ACMR` + (no deleted paths), stricter `set -euo pipefail`, portable Markdown invocation (no GNU `xargs -r`), and + safe iteration for “safe change” detection and version-source checks. --- diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 22817115..9a8e14f1 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -79,8 +79,8 @@ hatch test --cover -v ### Pre-commit Checks Local hooks use **`fail_fast: true`** and a **modular layout** aligned with `specfact-cli-modules`: -branch-aware module verify (skip if no staged module tree changes; `--allow-unsigned` except on - `main`, where `--require-signature`) → sync version files when those paths are staged → format (always) → +branch-aware module verify (skip if no staged module tree changes; on `main` pass `--require-signature` + to `verify-modules-signature.py`, elsewhere omit it for checksum-only mode) → sync version files when those paths are staged → format (always) → YAML / Markdown / workflow lint when matching paths are staged → **`hatch run lint`** when Python is staged → Block 2 (scoped code review + contract tests, with a safe-change short-circuit for docs-only and similar commits). See `.pre-commit-config.yaml` and `scripts/pre-commit-quality-checks.sh`. diff --git a/scripts/git-branch-module-signature-flag.sh b/scripts/git-branch-module-signature-flag.sh index 02d97904..0d5f9565 100755 --- a/scripts/git-branch-module-signature-flag.sh +++ b/scripts/git-branch-module-signature-flag.sh @@ -1,6 +1,7 @@ #!/usr/bin/env bash -# Emit verify-modules-signature.py signature policy flag for the current git branch. -# Prints --require-signature on main; --allow-unsigned elsewhere (including detached HEAD). +# Emit module signature policy for the current git branch (consumed by pre-commit-verify-modules.sh). +# Prints a single token: "require" on main (pass --require-signature to verify-modules-signature.py); +# "omit" elsewhere (verifier defaults to checksum-only; there is no --allow-unsigned CLI flag). set -euo pipefail branch="" @@ -9,7 +10,7 @@ if [[ -z "${branch}" || "${branch}" == "HEAD" ]]; then branch=$(git rev-parse --abbrev-ref HEAD 2>/dev/null || true) fi if [[ "${branch}" == "main" ]]; then - printf '%s\n' "--require-signature" + printf '%s\n' "require" else - printf '%s\n' "--allow-unsigned" + printf '%s\n' "omit" fi diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index c8724130..0459a441 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -11,7 +11,7 @@ # specfact-cli-modules check-bundle-imports). Module signature verification is a separate # pre-commit hook in .pre-commit-config.yaml, matching the modules repo. -set -e +set -euo pipefail RED='\033[0;31m' GREEN='\033[0;32m' @@ -44,34 +44,68 @@ print_block2_overview() { } staged_files() { - git diff --cached --name-only + git diff --cached --name-only --diff-filter=ACMR } has_staged_yaml() { - staged_files | grep -E '\.ya?ml$' >/dev/null 2>&1 + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + if [[ "${line}" =~ \.(yaml|yml)$ ]]; then + return 0 + fi + done < <(staged_files) + return 1 } has_staged_workflows() { - staged_files | grep -E '^\.github/workflows/.*\.ya?ml$' >/dev/null 2>&1 + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + if [[ "${line}" =~ ^\.github/workflows/.*\.ya?ml$ ]]; then + return 0 + fi + done < <(staged_files) + return 1 } has_staged_markdown() { - staged_files | grep -E '\.md$' >/dev/null 2>&1 + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + if [[ "${line}" =~ \.md$ ]]; then + return 0 + fi + done < <(staged_files) + return 1 } has_staged_python() { - staged_files | grep -E '\.pyi?$' >/dev/null 2>&1 + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + if [[ "${line}" =~ \.(py|pyi)$ ]]; then + return 0 + fi + done < <(staged_files) + return 1 } staged_markdown_files() { - staged_files | grep -E '\.md$' || true + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + if [[ "${line}" =~ \.md$ ]]; then + printf '%s\n' "${line}" + fi + done < <(staged_files) } # Paths eligible for the code review gate (parity with modules: scoped prefixes; non-Python filtered by pre_commit_code_review.py). staged_review_gate_files() { local line - while IFS= read -r line; do - [ -z "${line}" ] && continue + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue case "${line}" in */TDD_EVIDENCE.md|TDD_EVIDENCE.md) continue ;; src/*|scripts/*|tools/*|tests/*|openspec/changes/*) @@ -82,35 +116,24 @@ staged_review_gate_files() { } fail_if_markdown_has_unstaged_hunks() { - local md_files - md_files=$(staged_markdown_files) - if [ -z "${md_files}" ]; then - return - fi local file - while IFS= read -r file; do - [ -z "${file}" ] && continue - if ! git diff --quiet -- "$file"; then - error "❌ Cannot auto-fix Markdown with unstaged hunks: $file" + while IFS= read -r file || [[ -n "${file}" ]]; do + [[ -z "${file}" ]] && continue + if ! git diff --quiet -- "${file}"; then + error "❌ Cannot auto-fix Markdown with unstaged hunks: ${file}" warn "💡 Stage the full file or stash/revert the unstaged Markdown changes before commit" exit 1 fi - done </dev/null 2>&1; then - if echo "${md_files}" | xargs -r markdownlint --fix --config .markdownlint.json; then - echo "${md_files}" | xargs -r git add -- + if markdownlint --fix --config .markdownlint.json "${md_files[@]}"; then + git add -- "${md_files[@]}" success "✅ Block 1 — Markdown auto-fix applied" else error "❌ Block 1 — Markdown auto-fix failed" exit 1 fi else - if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --fix --config .markdownlint.json; then - echo "${md_files}" | xargs -r git add -- + if npx --yes markdownlint-cli --fix --config .markdownlint.json "${md_files[@]}"; then + git add -- "${md_files[@]}" success "✅ Block 1 — Markdown auto-fix applied (npx)" else error "❌ Block 1 — Markdown auto-fix failed (npx)" @@ -243,20 +269,20 @@ run_markdown_lint_if_needed() { return fi info "📦 Block 1 — Markdown lint — running markdownlint" - local md_files - md_files=$(staged_markdown_files) - if [ -z "${md_files}" ]; then + local md_files=() + mapfile -t md_files < <(staged_markdown_files) + if ((${#md_files[@]} == 0)); then return fi if command -v markdownlint >/dev/null 2>&1; then - if echo "${md_files}" | xargs -r markdownlint --config .markdownlint.json; then + if markdownlint --config .markdownlint.json "${md_files[@]}"; then success "✅ Block 1 — Markdown lint passed" else error "❌ Block 1 — Markdown lint failed" exit 1 fi else - if echo "${md_files}" | xargs -r npx --yes markdownlint-cli --config .markdownlint.json; then + if npx --yes markdownlint-cli --config .markdownlint.json "${md_files[@]}"; then success "✅ Block 1 — Markdown lint passed (npx)" else error "❌ Block 1 — Markdown lint failed (npx)" @@ -296,8 +322,8 @@ run_lint_if_staged_python() { run_code_review_gate() { local review_array=() - while IFS= read -r line; do - [ -z "${line}" ] && continue + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue review_array+=("${line}") done < <(staged_review_gate_files) diff --git a/scripts/pre-commit-smart-checks.sh b/scripts/pre-commit-smart-checks.sh index 3cc69737..a27d7c44 100755 --- a/scripts/pre-commit-smart-checks.sh +++ b/scripts/pre-commit-smart-checks.sh @@ -1,5 +1,17 @@ #!/usr/bin/env bash # Back-compat entry: single hook for downstream repos that pin `specfact-smart-checks`. # Canonical layout is modular hooks in .pre-commit-config.yaml → pre-commit-quality-checks.sh. +# +# Resolves the quality script from the repository root so copies under .git/hooks/pre-commit work. set -euo pipefail -exec "$(dirname "$0")/pre-commit-quality-checks.sh" all "$@" + +_script_path=${BASH_SOURCE[0]} +case "${_script_path}" in + /*) ;; + *) _script_path=$(pwd)/${_script_path} ;; +esac +_hook_dir=$(CDPATH= cd -- "$(dirname "${_script_path}")" && pwd) + +_repo_root=$(git -C "${_hook_dir}" rev-parse --show-toplevel) + +exec bash "${_repo_root}/scripts/pre-commit-quality-checks.sh" all "$@" diff --git a/scripts/pre-commit-verify-modules.sh b/scripts/pre-commit-verify-modules.sh index a30d705d..0a44dee3 100755 --- a/scripts/pre-commit-verify-modules.sh +++ b/scripts/pre-commit-verify-modules.sh @@ -10,7 +10,7 @@ if [[ -z "${repo_root}" ]]; then fi cd "${repo_root}" -staged_files=$(git diff --cached --name-only 2>/dev/null || true) +staged_files=$(git diff --cached --name-only --diff-filter=ACMR 2>/dev/null || true) if ! echo "${staged_files}" | grep -qE '^(src/specfact_cli/modules|modules)/'; then echo "ℹ️ No staged changes under modules/ or src/specfact_cli/modules/ — skipping module signature verification" exit 0 @@ -21,6 +21,10 @@ if [[ ! -f "${flag_script}" ]]; then echo "❌ Missing ${flag_script}" >&2 exit 1 fi -sig_flag=$(bash "${flag_script}") -echo "🔐 Verifying bundled module manifests (${sig_flag}, --enforce-version-bump, --payload-from-filesystem)" >&2 -exec hatch run ./scripts/verify-modules-signature.py "${sig_flag}" --enforce-version-bump --payload-from-filesystem +sig_policy=$(bash "${flag_script}") +if [[ "${sig_policy}" == "require" ]]; then + echo "🔐 Verifying bundled module manifests (--require-signature, --enforce-version-bump, --payload-from-filesystem)" >&2 + exec hatch run ./scripts/verify-modules-signature.py --require-signature --enforce-version-bump --payload-from-filesystem +fi +echo "🔐 Verifying bundled module manifests (checksum-only; --enforce-version-bump, --payload-from-filesystem)" >&2 +exec hatch run ./scripts/verify-modules-signature.py --enforce-version-bump --payload-from-filesystem diff --git a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py index d6fa4185..65572336 100644 --- a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py +++ b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py @@ -24,12 +24,12 @@ def test_pre_commit_markdown_checks_run_autofix_before_lint() -> None: def test_pre_commit_markdown_autofix_restages_files() -> None: script = _quality_script_text() - assert "xargs -r git add --" in script + assert 'git add -- "${md_files[@]}"' in script def test_pre_commit_markdown_autofix_rejects_partial_staging() -> None: script = _quality_script_text() - assert 'git diff --quiet -- "$file"' in script + assert 'git diff --quiet -- "${file}"' in script assert "Cannot auto-fix Markdown with unstaged hunks" in script @@ -50,3 +50,5 @@ def test_pre_commit_smart_checks_shim_delegates_to_quality_all() -> None: shim = _smart_shim_text() assert "pre-commit-quality-checks.sh" in shim assert 'all "$@"' in shim + assert "rev-parse --show-toplevel" in shim + assert 'exec bash "${_repo_root}/scripts/pre-commit-quality-checks.sh"' in shim diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py index 26ef94a2..472b18a0 100644 --- a/tests/unit/scripts/test_pre_commit_verify_modules.py +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -19,6 +19,9 @@ def test_verify_wrapper_invokes_branch_flag_and_payload_from_filesystem() -> Non assert "--payload-from-filesystem" in body assert "--enforce-version-bump" in body assert "verify-modules-signature.py" in body + assert 'sig_policy=$(bash "${flag_script}")' in body + assert '[[ "${sig_policy}" == "require" ]]' in body + assert "--require-signature" in body def _run_flag(*, cwd: Path) -> str: @@ -58,9 +61,9 @@ def _git_init_with_commit(repo: Path) -> None: @pytest.mark.parametrize( ("branch", "expected"), ( - ("feature/foo", "--allow-unsigned"), - ("dev", "--allow-unsigned"), - ("main", "--require-signature"), + ("feature/foo", "omit"), + ("dev", "omit"), + ("main", "require"), ), ) def test_git_branch_signature_flag(tmp_path: Path, branch: str, expected: str) -> None: @@ -69,3 +72,17 @@ def test_git_branch_signature_flag(tmp_path: Path, branch: str, expected: str) - _git_init_with_commit(repo) subprocess.run(["git", "branch", "-M", branch], cwd=repo, check=True, capture_output=True, text=True) assert _run_flag(cwd=repo) == expected + + +def test_git_branch_signature_flag_detached_head(tmp_path: Path) -> None: + repo = tmp_path / "repo" + repo.mkdir() + _git_init_with_commit(repo) + subprocess.run( + ["git", "checkout", "--detach", "HEAD"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + assert _run_flag(cwd=repo) == "omit" From 8dba304e49410331089a2ab66f6d5649a9373548 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 19:25:03 +0200 Subject: [PATCH 4/8] docs: align signing and verification docs with verifier CLI - Document checksum-only vs --require-signature; clarify --allow-unsigned is sign-modules.py only - Add pre-commit and CI branch policy to module-security, signing guide, publishing, agent gates - Refresh marketplace-06 OpenSpec proposal/design/tasks/spec delta; openspec validate --strict OK - CHANGELOG: note doc and OpenSpec alignment Made-with: Cursor --- CHANGELOG.md | 4 +++ .../50-quality-gates-and-review.md | 17 +++++++--- .../guides/module-signing-and-key-rotation.md | 29 ++++++++++++++--- docs/guides/publishing-modules.md | 5 ++- docs/reference/module-security.md | 15 +++++++-- .../design.md | 9 ++++-- .../proposal.md | 17 ++++++---- .../specs/ci-integration/spec.md | 7 +++-- .../marketplace-06-ci-module-signing/tasks.md | 31 +++++++++---------- 9 files changed, 94 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 97ef7cee..7f5bd378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,10 @@ All notable changes to this project will be documented in this file. - **`scripts/pre-commit-quality-checks.sh`**: staged file enumeration uses `git diff --cached --diff-filter=ACMR` (no deleted paths), stricter `set -euo pipefail`, portable Markdown invocation (no GNU `xargs -r`), and safe iteration for “safe change” detection and version-source checks. +- **Docs / OpenSpec**: `docs/reference/module-security.md`, `docs/guides/module-signing-and-key-rotation.md`, + `docs/guides/publishing-modules.md`, and `docs/agent-rules/50-quality-gates-and-review.md` now describe + branch-aware verify vs strict `--require-signature`, and clarify that `--allow-unsigned` applies to + `sign-modules.py` only; `openspec/changes/marketplace-06-ci-module-signing/` artifacts updated to match. --- diff --git a/docs/agent-rules/50-quality-gates-and-review.md b/docs/agent-rules/50-quality-gates-and-review.md index 373e94cb..28bb5379 100644 --- a/docs/agent-rules/50-quality-gates-and-review.md +++ b/docs/agent-rules/50-quality-gates-and-review.md @@ -14,7 +14,7 @@ tracks: - scripts/pre_commit_code_review.py - scripts/verify-modules-signature.py - docs/agent-rules/** -last_reviewed: 2026-04-10 +last_reviewed: 2026-04-14 exempt: false exempt_reason: "" id: agent-rules-quality-gates-and-review @@ -59,10 +59,19 @@ The repository enforces the clean-code charter through `specfact code review run ## Module signature gate -Before PR creation, every change that affects signed module assets or manifests must pass: +Every change that affects signed module assets or bundled manifests must satisfy verification **before +the change reaches `main`**. + +- **Local / feature branches**: pre-commit may run `verify-modules-signature.py` **without** + `--require-signature` (checksum-only) when only `dev` or a feature branch is checked out — see + `scripts/pre-commit-verify-modules.sh` and `scripts/git-branch-module-signature-flag.sh`. +- **Before merging to `main` or when validating release readiness**, run strict verification: ```bash -hatch run ./scripts/verify-modules-signature.py --require-signature +hatch run ./scripts/verify-modules-signature.py --require-signature --enforce-version-bump ``` -If verification fails because module contents changed, re-sign the affected manifests and bump the module version before re-running verification. +If verification fails because module contents changed, re-sign the affected manifests and bump the +module version before re-running verification. Note: `verify-modules-signature.py` has **no** +`--allow-unsigned` flag; checksum-only mode is “omit `--require-signature`”. The `--allow-unsigned` +option on **`sign-modules.py`** is only for local test signing. diff --git a/docs/guides/module-signing-and-key-rotation.md b/docs/guides/module-signing-and-key-rotation.md index 4d845cee..c4b37367 100644 --- a/docs/guides/module-signing-and-key-rotation.md +++ b/docs/guides/module-signing-and-key-rotation.md @@ -120,14 +120,35 @@ With explicit public key file: python scripts/verify-modules-signature.py --require-signature --public-key-file resources/keys/module-signing-public.pem ``` +Checksum and version discipline without requiring signatures (same tool; omit the flag): + +```bash +hatch run python scripts/verify-modules-signature.py --enforce-version-bump --payload-from-filesystem +``` + +Do not pass `--allow-unsigned` to `verify-modules-signature.py` — it is not a supported argument there. +Use `python scripts/sign-modules.py --allow-unsigned …` only when you intentionally want checksum-only +**signing** for local tests. + +## Pre-commit (bundled modules in this repository) + +If you use `pre-commit` or `scripts/setup-git-hooks.sh`, commits that stage changes under `modules/` or +`src/specfact_cli/modules/` run `scripts/pre-commit-verify-modules.sh`. That script adds +`--require-signature` only when the current branch is `main`; on other branches (including detached +`HEAD`) it runs checksum-only verification so commits do not require a local private key. + ## CI Enforcement -`pr-orchestrator.yml` contains a strict gate: +`pr-orchestrator.yml` runs job `verify-module-signatures` with a **branch-aware** policy: -- Job: `verify-module-signatures` -- Command: `python scripts/verify-modules-signature.py --require-signature` +- PRs and pushes targeting **`main`**: `verify-modules-signature.py` is invoked **with** + `--require-signature` (plus `--enforce-version-bump --payload-from-filesystem` and PR base comparison + as configured in the workflow). +- PRs and pushes targeting **`dev`**: the same script runs **without** `--require-signature` + (checksum-only), matching local feature-branch development. -This runs on PR/push for `dev` and `main` and fails the pipeline if module signatures/checksums are missing or stale. +The pipeline fails if checksums or version-bump rules are violated, or if `main`-targeting events lack +valid signatures when required. ## Rotation Procedure diff --git a/docs/guides/publishing-modules.md b/docs/guides/publishing-modules.md index 6ac5adcf..575e6597 100644 --- a/docs/guides/publishing-modules.md +++ b/docs/guides/publishing-modules.md @@ -104,7 +104,10 @@ metadata. - Bump module `version` in `module-package.yaml` whenever payload or manifest content changes; keep versions immutable for published artifacts. - Use `namespace/name` for any module you publish to a registry. -- Run `scripts/verify-modules-signature.py --require-signature` (or your registry’s policy) before releasing. +- Before releasing from a protected branch, run strict verification, e.g. + `scripts/verify-modules-signature.py --require-signature` (checksum-only is the default when that + flag is omitted — see [Module signing and key rotation](module-signing-and-key-rotation.md)). Follow + your registry’s policy if stricter. - Prefer `--download-base-url` and `--index-fragment` when integrating with a custom registry index. ## See also diff --git a/docs/reference/module-security.md b/docs/reference/module-security.md index 32a51422..8a889f50 100644 --- a/docs/reference/module-security.md +++ b/docs/reference/module-security.md @@ -46,9 +46,18 @@ Module packages carry **publisher** and **integrity** metadata so installation, - **CI secrets**: - `SPECFACT_MODULE_PRIVATE_SIGN_KEY` - `SPECFACT_MODULE_PRIVATE_SIGN_KEY_PASSPHRASE` -- **Verification command**: - - `scripts/verify-modules-signature.py --require-signature --enforce-version-bump` - - `--version-check-base ` can be used in CI PR comparisons. +- **Verification command** (`verify-modules-signature.py`): + - **Strict** (signatures required): `--require-signature --enforce-version-bump` (and optional + `--payload-from-filesystem`, `--version-check-base ` in CI). + - **Checksum-only** (default when `--require-signature` is omitted): still enforces payload + checksums and, with `--enforce-version-bump`, version discipline — useful on feature branches and + for dev-targeting CI without local signing keys. + - There is **no** `--allow-unsigned` on this verifier; that flag exists on **`sign-modules.py`** + for explicit test-only signing without a key. +- **Pre-commit** (this repo): when staged paths exist under `modules/` or `src/specfact_cli/modules/`, + `scripts/pre-commit-verify-modules.sh` runs the verifier with `--enforce-version-bump` and + `--payload-from-filesystem`, adding `--require-signature` only on `main` (see + `scripts/git-branch-module-signature-flag.sh`). ## Public key and key rotation diff --git a/openspec/changes/marketplace-06-ci-module-signing/design.md b/openspec/changes/marketplace-06-ci-module-signing/design.md index ffe85a72..db1de46e 100644 --- a/openspec/changes/marketplace-06-ci-module-signing/design.md +++ b/openspec/changes/marketplace-06-ci-module-signing/design.md @@ -81,10 +81,13 @@ workflow. If stricter loop prevention is needed, the commit message includes `[s `--changed-only` detects no payload change since the last sign commit and skips. The resulting manifest is byte-for-byte identical due to deterministic YAML serialisation. -### Decision 3: Branch-aware pre-commit — `--allow-unsigned` on non-`main` +### Decision 3: Branch-aware pre-commit — omit `--require-signature` off `main` -**Chosen**: Detect current branch via `git branch --show-current`. If not `main`, call -`verify-modules-signature.py` with `--allow-unsigned`; on `main` retain `--require-signature`. +**Chosen**: `scripts/git-branch-module-signature-flag.sh` emits `require` on `main` and `omit` elsewhere +(including detached `HEAD`). `scripts/pre-commit-verify-modules.sh` passes `--require-signature` to +`verify-modules-signature.py` only when the policy is `require`; otherwise it invokes the same script +without that flag so verification stays checksum-only. There is **no** `--allow-unsigned` on +`verify-modules-signature.py` (that flag belongs to **`sign-modules.py`** for explicit test signing). **Rationale**: Removes the local key requirement for all development work. Developers and agents on feature or dev branches can commit freely. The `main` guard is a secondary defence; the primary diff --git a/openspec/changes/marketplace-06-ci-module-signing/proposal.md b/openspec/changes/marketplace-06-ci-module-signing/proposal.md index c4941e0f..1aea7b19 100644 --- a/openspec/changes/marketplace-06-ci-module-signing/proposal.md +++ b/openspec/changes/marketplace-06-ci-module-signing/proposal.md @@ -15,8 +15,12 @@ while preserving the integrity guarantee where it matters: at the trust boundary - **NEW**: `sign-modules-on-approval.yml` GitHub Actions workflow — triggers on `pull_request_review` (state: `approved`), signs changed module manifests via CI secrets, and commits the signed manifests back to the PR branch. -- **MODIFY**: `scripts/pre-commit-smart-checks.sh` — branch-aware signature policy: non-`main` - branches use `--allow-unsigned` (checksum-only); `main` branch retains `--require-signature`. +- **MODIFY**: Pre-commit module verify — branch-aware policy via `scripts/pre-commit-verify-modules.sh` + and `scripts/git-branch-module-signature-flag.sh`: on non-`main` branches (including detached `HEAD`), + run `verify-modules-signature.py` **without** `--require-signature` (checksum-only); on `main`, pass + `--require-signature`. The verifier has **no** `--allow-unsigned` flag (that option exists on + **`sign-modules.py`** for local test signing only). `scripts/pre-commit-smart-checks.sh` remains a + repo-root shim into `pre-commit-quality-checks.sh` (see modular `.pre-commit-config.yaml`). - **MODIFY**: `.github/workflows/pr-orchestrator.yml` `verify-module-signatures` job — drop `--require-signature` for PRs and pushes targeting `dev`; keep it for PRs and pushes targeting `main`. @@ -36,13 +40,14 @@ while preserving the integrity guarantee where it matters: at the trust boundary ### Modified Capabilities -- `ci-integration`: Pre-commit and CI verification gates now apply a branch-aware policy — - `--allow-unsigned` (checksum-only) on non-`main` branches and in feature/dev PRs; full - `--require-signature` enforcement only on `main`-targeting PRs and `main` branch pushes. +- `ci-integration`: Pre-commit and CI verification gates apply a branch-aware policy — omit + `--require-signature` (checksum-only) on non-`main` branches and for dev-targeting PR/push events; + pass `--require-signature` only on `main` and for `main`-targeting PR/push events. ## Impact -- **Affected scripts**: `scripts/pre-commit-smart-checks.sh` +- **Affected scripts**: `scripts/pre-commit-verify-modules.sh`, `scripts/git-branch-module-signature-flag.sh`, + `scripts/pre-commit-quality-checks.sh`, `scripts/pre-commit-smart-checks.sh` (shim) - **Affected workflows**: `.github/workflows/pr-orchestrator.yml`, `.github/workflows/sign-modules.yml` - **New workflow**: `.github/workflows/sign-modules-on-approval.yml` diff --git a/openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md b/openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md index d71cea19..35abc2ec 100644 --- a/openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md +++ b/openspec/changes/marketplace-06-ci-module-signing/specs/ci-integration/spec.md @@ -9,9 +9,10 @@ non-`main` branches, full signature verification on `main`. #### Scenario: Pre-commit on feature or dev branch without local key -- **WHEN** a developer or agent runs `git commit` on any branch other than `main` -- **AND** the commit includes changes to module files -- **THEN** the pre-commit hook SHALL run `verify-modules-signature.py --allow-unsigned` +- **WHEN** a developer or agent runs `git commit` on any branch other than `main` (or on detached `HEAD`) +- **AND** the commit includes staged changes under `modules/` or `src/specfact_cli/modules/` +- **THEN** the pre-commit hook SHALL run `verify-modules-signature.py` with `--enforce-version-bump` + and `--payload-from-filesystem` **without** `--require-signature` (checksum-only default) - **AND** SHALL accept manifests with a valid checksum but no signature - **AND** SHALL NOT fail due to a missing or invalid signature diff --git a/openspec/changes/marketplace-06-ci-module-signing/tasks.md b/openspec/changes/marketplace-06-ci-module-signing/tasks.md index 6431c96d..ceb7025a 100644 --- a/openspec/changes/marketplace-06-ci-module-signing/tasks.md +++ b/openspec/changes/marketplace-06-ci-module-signing/tasks.md @@ -10,35 +10,34 @@ ## 2. Specs and TDD evidence (failing tests first) -- [ ] 2.1 Write unit tests for the pre-commit branch-detection logic in - `tests/unit/scripts/test_pre_commit_module_signing.py` covering: non-main branch accepts unsigned, - main branch rejects unsigned, no module changes passes without check. Run and capture failing - output in `TDD_EVIDENCE.md`. +- [ ] 2.1 Write unit tests for branch policy (`scripts/git-branch-module-signature-flag.sh` and + `pre-commit-verify-modules.sh` wiring), e.g. under `tests/unit/scripts/`, covering: non-main omits + `--require-signature`, main requires signature, detached `HEAD` matches non-main policy, no staged + module paths skips verify. Run and capture failing output in `TDD_EVIDENCE.md`. - [ ] 2.2 Write integration tests (or workflow-syntax tests) for the signing workflow YAML structure in `tests/unit/workflows/test_sign_modules_on_approval.py` — validate trigger config, required env vars, and commit-back step presence. Capture failing output in `TDD_EVIDENCE.md`. - [ ] 2.3 Write tests for the updated `pr-orchestrator.yml` `verify-module-signatures` logic - confirming the branch split (`--allow-unsigned` for dev, `--require-signature` for main). Capture - failing output in `TDD_EVIDENCE.md`. + confirming the branch split (omit `--require-signature` for dev, pass `--require-signature` for main). + Capture failing output in `TDD_EVIDENCE.md`. ## 3. Pre-commit hook — branch-aware verification -- [ ] 3.1 In `scripts/pre-commit-smart-checks.sh`, refactor `run_module_signature_verification()` - to detect the current branch via `git branch --show-current` (fallback: `git rev-parse - --abbrev-ref HEAD`). -- [ ] 3.2 Apply policy: if branch is NOT `main`, call - `hatch run ./scripts/verify-modules-signature.py --allow-unsigned --enforce-version-bump`; - if branch IS `main`, keep the existing `--require-signature --enforce-version-bump` call. -- [ ] 3.3 Run the TDD unit tests from 2.1 and confirm they pass; record passing run in - `TDD_EVIDENCE.md`. +- [ ] 3.1 Implement branch policy in `scripts/git-branch-module-signature-flag.sh` (`require` on `main`, + `omit` elsewhere) and wire `scripts/pre-commit-verify-modules.sh` to pass `--require-signature` only + when policy is `require`; always pass `--enforce-version-bump --payload-from-filesystem` when the + hook runs. Skip the hook when no staged paths under `modules/` or `src/specfact_cli/modules/`. +- [ ] 3.2 Register the verify script in `.pre-commit-config.yaml` and ensure `pre-commit-quality-checks.sh` + `all` invokes module verification (modular hooks + `pre-commit-smart-checks.sh` repo-root shim as needed). +- [ ] 3.3 Run the TDD unit tests from 2.1 and confirm they pass; record passing run in `TDD_EVIDENCE.md`. ## 4. pr-orchestrator.yml — split verify by target branch - [ ] 4.1 In `.github/workflows/pr-orchestrator.yml`, in the `verify-module-signatures` job, add branch-target detection: extract `github.event.pull_request.base.ref` (for PR events) and `github.ref` (for push events). -- [ ] 4.2 For events targeting `dev` (PR base = `dev` or push ref = `refs/heads/dev`): replace - `--require-signature` with no flag (or explicit `--allow-unsigned` equivalent); keep +- [ ] 4.2 For events targeting `dev` (PR base = `dev` or push ref = `refs/heads/dev`): omit + `--require-signature` from the verifier invocation (checksum-only); keep `--enforce-version-bump --payload-from-filesystem`. - [ ] 4.3 For events targeting `main` (PR base = `main` or push ref = `refs/heads/main`): retain `--require-signature --enforce-version-bump --payload-from-filesystem`. From 1eee45de85cb482d41b9903c21f213578935b9bd Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 19:51:24 +0200 Subject: [PATCH 5/8] =?UTF-8?q?fix(pre-commit):=20address=20review=20?= =?UTF-8?q?=E2=80=94=20sig=5Fpolicy=20guard,=20DRY=20contract=20check,=20t?= =?UTF-8?q?ests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Validate require|omit from git-branch-module-signature-flag; error on unknown policy - check_contract_script_exists for tools/contract_first_smart_test.py (run_block2 + run_all) - Comment why contract-test-status stdout/stderr are discarded - Tests: run_all-scoped markdown order; fake hatch integration for verify wrapper; 8s flag timeout Made-with: Cursor --- CHANGELOG.md | 3 + scripts/pre-commit-quality-checks.sh | 19 +-- scripts/pre-commit-verify-modules.sh | 22 ++- .../test_pre_commit_smart_checks_docs.py | 11 +- .../scripts/test_pre_commit_verify_modules.py | 130 ++++++++++++++++-- 5 files changed, 157 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7f5bd378..a791a5fc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,9 @@ All notable changes to this project will be documented in this file. `docs/guides/publishing-modules.md`, and `docs/agent-rules/50-quality-gates-and-review.md` now describe branch-aware verify vs strict `--require-signature`, and clarify that `--allow-unsigned` applies to `sign-modules.py` only; `openspec/changes/marketplace-06-ci-module-signing/` artifacts updated to match. +- **Pre-commit follow-ups**: `pre-commit-verify-modules.sh` fails closed on unexpected `sig_policy` output; + `pre-commit-quality-checks.sh` documents suppressed `contract-test-status` output and deduplicates the + contract-first script existence check; script tests use a fake `hatch` and tighter timeouts. --- diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index 0459a441..e242d215 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -344,6 +344,8 @@ run_code_review_gate() { run_contract_tests_visible() { info "📦 Block 2 — contract tests — running \`hatch run contract-test-status\`" + # Discard status-check output: transient failures (missing optional deps, environment noise) should + # not alarm the user; we fall through to the full `hatch run contract-test` which surfaces real failures. if hatch run contract-test-status >/dev/null 2>&1; then success "✅ Block 2 — contract tests — skipped (contract-test-status: no input changes)" else @@ -359,6 +361,13 @@ run_contract_tests_visible() { fi } +check_contract_script_exists() { + if [[ ! -f "tools/contract_first_smart_test.py" ]]; then + error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" + exit 1 + fi +} + run_block1_format() { warn "🔍 specfact-cli pre-commit — Block 1 — hook: format" print_block1_overview @@ -399,10 +408,7 @@ run_block2() { fi print_block2_overview run_code_review_gate - if [ ! -f "tools/contract_first_smart_test.py" ]; then - error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" - exit 1 - fi + check_contract_script_exists run_contract_tests_visible } @@ -424,10 +430,7 @@ run_all() { fi print_block2_overview run_code_review_gate - if [ ! -f "tools/contract_first_smart_test.py" ]; then - error "❌ Contract-first test script not found. Please run: hatch run contract-test-full" - exit 1 - fi + check_contract_script_exists run_contract_tests_visible } diff --git a/scripts/pre-commit-verify-modules.sh b/scripts/pre-commit-verify-modules.sh index 0a44dee3..8cab75af 100755 --- a/scripts/pre-commit-verify-modules.sh +++ b/scripts/pre-commit-verify-modules.sh @@ -22,9 +22,19 @@ if [[ ! -f "${flag_script}" ]]; then exit 1 fi sig_policy=$(bash "${flag_script}") -if [[ "${sig_policy}" == "require" ]]; then - echo "🔐 Verifying bundled module manifests (--require-signature, --enforce-version-bump, --payload-from-filesystem)" >&2 - exec hatch run ./scripts/verify-modules-signature.py --require-signature --enforce-version-bump --payload-from-filesystem -fi -echo "🔐 Verifying bundled module manifests (checksum-only; --enforce-version-bump, --payload-from-filesystem)" >&2 -exec hatch run ./scripts/verify-modules-signature.py --enforce-version-bump --payload-from-filesystem +sig_policy="${sig_policy//$'\r'/}" +sig_policy="${sig_policy//$'\n'/}" +case "${sig_policy}" in + require) + echo "🔐 Verifying bundled module manifests (--require-signature, --enforce-version-bump, --payload-from-filesystem)" >&2 + exec hatch run ./scripts/verify-modules-signature.py --require-signature --enforce-version-bump --payload-from-filesystem + ;; + omit) + echo "🔐 Verifying bundled module manifests (checksum-only; --enforce-version-bump, --payload-from-filesystem)" >&2 + exec hatch run ./scripts/verify-modules-signature.py --enforce-version-bump --payload-from-filesystem + ;; + *) + echo "❌ Invalid module signature policy from ${flag_script}: '${sig_policy}' (expected require or omit)" >&2 + exit 1 + ;; +esac diff --git a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py index 65572336..a7d1fc4c 100644 --- a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py +++ b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py @@ -17,9 +17,14 @@ def test_pre_commit_markdown_checks_run_autofix_before_lint() -> None: script = _quality_script_text() assert "run_markdown_autofix_if_needed" in script assert "markdownlint --fix --config .markdownlint.json" in script - idx_fix = script.find("run_markdown_autofix_if_needed") - idx_lint = script.find("run_markdown_lint_if_needed") - assert 0 <= idx_fix < idx_lint, "auto-fix must be defined before lint in the script" + start = script.find("run_all()") + assert start != -1 + end = script.find("\nusage_error()", start) + assert end != -1 + run_all_block = script[start:end] + idx_fix = run_all_block.find("run_markdown_autofix_if_needed") + idx_lint = run_all_block.find("run_markdown_lint_if_needed") + assert 0 <= idx_fix < idx_lint, "auto-fix must run before lint inside run_all()" def test_pre_commit_markdown_autofix_restages_files() -> None: diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py index 472b18a0..07bd2e76 100644 --- a/tests/unit/scripts/test_pre_commit_verify_modules.py +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -2,6 +2,8 @@ from __future__ import annotations +import os +import stat import subprocess from pathlib import Path @@ -12,16 +14,10 @@ FLAG_SCRIPT = REPO_ROOT / "scripts" / "git-branch-module-signature-flag.sh" VERIFY_WRAPPER = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" - -def test_verify_wrapper_invokes_branch_flag_and_payload_from_filesystem() -> None: - body = VERIFY_WRAPPER.read_text(encoding="utf-8") - assert "git-branch-module-signature-flag.sh" in body - assert "--payload-from-filesystem" in body - assert "--enforce-version-bump" in body - assert "verify-modules-signature.py" in body - assert 'sig_policy=$(bash "${flag_script}")' in body - assert '[[ "${sig_policy}" == "require" ]]' in body - assert "--require-signature" in body +TOKEN_VERIFY_SCRIPT = "verify-modules-signature.py" +TOKEN_REQUIRE_SIGNATURE = "--require-signature" +TOKEN_ENFORCE_VERSION_BUMP = "--enforce-version-bump" +TOKEN_PAYLOAD_FROM_FS = "--payload-from-filesystem" def _run_flag(*, cwd: Path) -> str: @@ -31,7 +27,7 @@ def _run_flag(*, cwd: Path) -> str: capture_output=True, text=True, check=False, - timeout=30, + timeout=8, ) assert result.returncode == 0, result.stderr return result.stdout.strip() @@ -58,6 +54,118 @@ def _git_init_with_commit(repo: Path) -> None: subprocess.run(["git", "commit", "-m", "init"], cwd=repo, check=True, capture_output=True, text=True) +def _write_fake_hatch(bin_dir: Path, log_path: Path) -> Path: + hatch = bin_dir / "hatch" + hatch.write_text( + f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"{log_path}\"\nexit 0\n", + encoding="utf-8", + ) + hatch.chmod(hatch.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + return hatch + + +def _repo_with_verify_scripts( + tmp_path: Path, + *, + flag_script_body: str | None = None, +) -> tuple[Path, Path]: + """Minimal git repo with staged module file and verify/flag scripts (symlink or custom flag).""" + repo = tmp_path / "repo" + scripts = repo / "scripts" + scripts.mkdir(parents=True) + (repo / "modules").mkdir() + (repo / "modules" / "pkg.yaml").write_text("x: 1\n", encoding="utf-8") + + (scripts / "pre-commit-verify-modules.sh").symlink_to(VERIFY_WRAPPER.resolve()) + flag_target = scripts / "git-branch-module-signature-flag.sh" + if flag_script_body is None: + flag_target.symlink_to(FLAG_SCRIPT.resolve()) + else: + flag_target.write_text(flag_script_body, encoding="utf-8") + flag_target.chmod(flag_target.stat().st_mode | stat.S_IXUSR) + + subprocess.run(["git", "init"], cwd=repo, check=True, capture_output=True, text=True) + subprocess.run( + ["git", "config", "user.email", "t@e.com"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + subprocess.run( + ["git", "config", "user.name", "T"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) + subprocess.run(["git", "add", "modules/pkg.yaml"], cwd=repo, check=True, capture_output=True, text=True) + log_path = tmp_path / "hatch_invocations.log" + bin_dir = tmp_path / "bin" + bin_dir.mkdir() + _write_fake_hatch(bin_dir, log_path) + return repo, log_path + + +def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path) + subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) + env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} + result = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, (result.stdout, result.stderr) + log = log_path.read_text(encoding="utf-8") + assert TOKEN_VERIFY_SCRIPT in log + assert TOKEN_ENFORCE_VERSION_BUMP in log + assert TOKEN_PAYLOAD_FROM_FS in log + assert TOKEN_REQUIRE_SIGNATURE in log + + +def test_verify_wrapper_runs_hatch_checksum_only_off_main(tmp_path: Path) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path) + subprocess.run(["git", "branch", "-M", "feature/x"], cwd=repo, check=True, capture_output=True, text=True) + env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} + result = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, (result.stdout, result.stderr) + log = log_path.read_text(encoding="utf-8") + assert TOKEN_VERIFY_SCRIPT in log + assert TOKEN_ENFORCE_VERSION_BUMP in log + assert TOKEN_PAYLOAD_FROM_FS in log + assert TOKEN_REQUIRE_SIGNATURE not in log + + +def test_verify_wrapper_rejects_invalid_sig_policy(tmp_path: Path) -> None: + bad_flag = '#!/usr/bin/env bash\nset -euo pipefail\necho bogus\n' + repo, _log_path = _repo_with_verify_scripts(tmp_path, flag_script_body=bad_flag) + subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) + env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} + result = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode != 0 + assert "Invalid module signature policy" in result.stderr + assert "bogus" in result.stderr + assert "expected require or omit" in result.stderr + + @pytest.mark.parametrize( ("branch", "expected"), ( From 31a6e95c7dacad662b785c1b3fabed4309c2ed8e Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 20:01:14 +0200 Subject: [PATCH 6/8] =?UTF-8?q?fix(pre-commit):=20review=20follow-ups=20?= =?UTF-8?q?=E2=80=94=20Block=202=20scope,=20git=20diff=20errors,=20skip=20?= =?UTF-8?q?test?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - check_safe_change: do not exempt pre-commit wrapper scripts from Block 2 when staged - pre-commit-verify-modules: fail if git diff --cached fails (no || true) - test: no-module-tree fast path; touch hatch log so skip path can assert empty - CHANGELOG: reflow + note git-diff failure handling and Block 2 exemption removal Made-with: Cursor --- CHANGELOG.md | 32 ++++++++------- scripts/pre-commit-quality-checks.sh | 1 - scripts/pre-commit-verify-modules.sh | 5 ++- .../scripts/test_pre_commit_verify_modules.py | 39 +++++++++++++++++-- 4 files changed, 56 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a791a5fc..dce41221 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,31 +14,33 @@ All notable changes to this project will be documented in this file. ### Added -- **`scripts/pre-commit-quality-checks.sh`**: modular Block 1/2 entrypoints (`block1-*`, `block2`, `all`) - with staged-file gates and Markdown auto-fix before lint (parity with `specfact-cli-modules` hook layout - and `fail_fast` behavior in `.pre-commit-config.yaml`). -- **`scripts/pre-commit-smart-checks.sh`**: back-compat shim that resolves the repository root (so copies - under `.git/hooks/pre-commit` still run the canonical quality script) and delegates to +- **`scripts/pre-commit-quality-checks.sh`**: modular Block 1/2 entrypoints (`block1-*`, `block2`, `all`) with + staged-file gates and Markdown auto-fix before lint (parity with `specfact-cli-modules` hook layout and + `fail_fast` behavior in `.pre-commit-config.yaml`). +- **`scripts/pre-commit-smart-checks.sh`**: back-compat shim that resolves the repository root (so copies under + `.git/hooks/pre-commit` still run the canonical quality script) and delegates to `pre-commit-quality-checks.sh all`. ### Changed - **Module verify (pre-commit)**: branch-aware policy via `scripts/pre-commit-verify-modules.sh` and `scripts/git-branch-module-signature-flag.sh` — on `main`, run `verify-modules-signature.py` with - `--require-signature`; on other branches (including detached `HEAD`), omit that flag so the verifier - stays in checksum-only mode (there is no `--allow-unsigned` CLI). Skips when no staged paths under - `modules/` or `src/specfact_cli/modules/`; when the check runs it always passes `--payload-from-filesystem` - and `--enforce-version-bump`. -- **`scripts/pre-commit-quality-checks.sh`**: staged file enumeration uses `git diff --cached --diff-filter=ACMR` - (no deleted paths), stricter `set -euo pipefail`, portable Markdown invocation (no GNU `xargs -r`), and - safe iteration for “safe change” detection and version-source checks. + `--require-signature`; on other branches (including detached `HEAD`), omit that flag so the verifier stays in + checksum-only mode (there is no `--allow-unsigned` CLI). Skips when no staged paths under `modules/` or + `src/specfact_cli/modules/`; when the check runs it always passes `--payload-from-filesystem` and + `--enforce-version-bump`. +- **`scripts/pre-commit-quality-checks.sh`**: staged file enumeration uses + `git diff --cached --diff-filter=ACMR` (no deleted paths), stricter `set -euo pipefail`, portable Markdown + invocation (no GNU `xargs -r`), and safe iteration for “safe change” detection and version-source checks; + pre-commit wrapper scripts are not exempt from Block 2 when staged. - **Docs / OpenSpec**: `docs/reference/module-security.md`, `docs/guides/module-signing-and-key-rotation.md`, `docs/guides/publishing-modules.md`, and `docs/agent-rules/50-quality-gates-and-review.md` now describe branch-aware verify vs strict `--require-signature`, and clarify that `--allow-unsigned` applies to `sign-modules.py` only; `openspec/changes/marketplace-06-ci-module-signing/` artifacts updated to match. -- **Pre-commit follow-ups**: `pre-commit-verify-modules.sh` fails closed on unexpected `sig_policy` output; - `pre-commit-quality-checks.sh` documents suppressed `contract-test-status` output and deduplicates the - contract-first script existence check; script tests use a fake `hatch` and tighter timeouts. +- **Pre-commit follow-ups**: `pre-commit-verify-modules.sh` fails closed on unexpected `sig_policy` output and + on `git diff --cached` errors; `pre-commit-quality-checks.sh` documents suppressed `contract-test-status` + output and deduplicates the contract-first script existence check; script tests use a fake `hatch`, tighter + timeouts, and cover the no-module-tree skip path. --- diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index e242d215..82d9f649 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -137,7 +137,6 @@ check_safe_change() { case "${file}" in pyproject.toml|setup.py|src/__init__.py|src/specfact_cli/__init__.py) ;; CHANGELOG.md|README.md|.pre-commit-config.yaml) ;; - scripts/pre-commit-quality-checks.sh|scripts/pre-commit-smart-checks.sh|scripts/pre-commit-verify-modules.sh|scripts/git-branch-module-signature-flag.sh) ;; tools/smart_test_coverage.py|tools/functional_coverage_analyzer.py) ;; *.md|*.rst|*.txt|*.json|*.yaml|*.yml) ;; docs/*|papers/*|presentations/*|images/*) ;; diff --git a/scripts/pre-commit-verify-modules.sh b/scripts/pre-commit-verify-modules.sh index 8cab75af..c468b3d2 100755 --- a/scripts/pre-commit-verify-modules.sh +++ b/scripts/pre-commit-verify-modules.sh @@ -10,7 +10,10 @@ if [[ -z "${repo_root}" ]]; then fi cd "${repo_root}" -staged_files=$(git diff --cached --name-only --diff-filter=ACMR 2>/dev/null || true) +staged_files=$(git diff --cached --name-only --diff-filter=ACMR) || { + echo "❌ Error discovering staged files (git diff --cached failed)" >&2 + exit 1 +} if ! echo "${staged_files}" | grep -qE '^(src/specfact_cli/modules|modules)/'; then echo "ℹ️ No staged changes under modules/ or src/specfact_cli/modules/ — skipping module signature verification" exit 0 diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py index 07bd2e76..12368a9d 100644 --- a/tests/unit/scripts/test_pre_commit_verify_modules.py +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -57,7 +57,7 @@ def _git_init_with_commit(repo: Path) -> None: def _write_fake_hatch(bin_dir: Path, log_path: Path) -> Path: hatch = bin_dir / "hatch" hatch.write_text( - f"#!/bin/sh\nprintf '%s\\n' \"$*\" >> \"{log_path}\"\nexit 0\n", + f'#!/bin/sh\nprintf \'%s\\n\' "$*" >> "{log_path}"\nexit 0\n', encoding="utf-8", ) hatch.chmod(hatch.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) @@ -68,8 +68,9 @@ def _repo_with_verify_scripts( tmp_path: Path, *, flag_script_body: str | None = None, + stage_module_paths: bool = True, ) -> tuple[Path, Path]: - """Minimal git repo with staged module file and verify/flag scripts (symlink or custom flag).""" + """Minimal git repo with verify/flag scripts; optionally stage paths under modules/.""" repo = tmp_path / "repo" scripts = repo / "scripts" scripts.mkdir(parents=True) @@ -99,14 +100,44 @@ def _repo_with_verify_scripts( capture_output=True, text=True, ) - subprocess.run(["git", "add", "modules/pkg.yaml"], cwd=repo, check=True, capture_output=True, text=True) + if stage_module_paths: + subprocess.run(["git", "add", "modules/pkg.yaml"], cwd=repo, check=True, capture_output=True, text=True) + else: + (repo / "README.md").write_text("seed\n", encoding="utf-8") + subprocess.run(["git", "add", "README.md"], cwd=repo, check=True, capture_output=True, text=True) + subprocess.run( + ["git", "commit", "-m", "init"], + cwd=repo, + check=True, + capture_output=True, + text=True, + ) log_path = tmp_path / "hatch_invocations.log" bin_dir = tmp_path / "bin" bin_dir.mkdir() _write_fake_hatch(bin_dir, log_path) + log_path.touch() return repo, log_path +def test_verify_wrapper_skips_when_no_module_paths_staged(tmp_path: Path) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path, stage_module_paths=False) + subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) + env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} + result = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, (result.stdout, result.stderr) + log = log_path.read_text(encoding="utf-8") + assert TOKEN_VERIFY_SCRIPT not in log + assert log.strip() == "", "fake hatch must not run when module tree paths are not staged" + + def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path) -> None: repo, log_path = _repo_with_verify_scripts(tmp_path) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) @@ -148,7 +179,7 @@ def test_verify_wrapper_runs_hatch_checksum_only_off_main(tmp_path: Path) -> Non def test_verify_wrapper_rejects_invalid_sig_policy(tmp_path: Path) -> None: - bad_flag = '#!/usr/bin/env bash\nset -euo pipefail\necho bogus\n' + bad_flag = "#!/usr/bin/env bash\nset -euo pipefail\necho bogus\n" repo, _log_path = _repo_with_verify_scripts(tmp_path, flag_script_body=bad_flag) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} From 24d17d792d7e942f5d6eec4d58dcc239c4bfe5b1 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 21:09:22 +0200 Subject: [PATCH 7/8] fix(pre-commit): classify changelog, harden format diff, extend verify tests - Move pre-commit follow-ups under 0.46.1 ### Fixed; note git diff exit >1 handling - run_format_safety: fail only when git diff exit code > 1 (keep diff=1 as success) - Test: fake git fails on diff --cached; skip-path uses staged docs/notes.txt only Made-with: Cursor --- CHANGELOG.md | 12 +++-- scripts/pre-commit-quality-checks.sh | 16 +++++-- .../scripts/test_pre_commit_verify_modules.py | 47 +++++++++++++++++-- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dce41221..c850fe8d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,14 @@ All notable changes to this project will be documented in this file. `.git/hooks/pre-commit` still run the canonical quality script) and delegates to `pre-commit-quality-checks.sh all`. +### Fixed + +- **Pre-commit robustness**: `pre-commit-verify-modules.sh` fails closed on unexpected `sig_policy` output and on + `git diff --cached` errors; `pre-commit-quality-checks.sh` documents suppressed `contract-test-status` output, + deduplicates the contract-first script existence check, and treats `git diff` exit codes greater than 1 as errors in + `run_format_safety` (exit 1 means “has diff”, not failure); script tests use a fake `hatch`, tighter timeouts, + skip-path and `git diff --cached` failure coverage. + ### Changed - **Module verify (pre-commit)**: branch-aware policy via `scripts/pre-commit-verify-modules.sh` and @@ -37,10 +45,6 @@ All notable changes to this project will be documented in this file. `docs/guides/publishing-modules.md`, and `docs/agent-rules/50-quality-gates-and-review.md` now describe branch-aware verify vs strict `--require-signature`, and clarify that `--allow-unsigned` applies to `sign-modules.py` only; `openspec/changes/marketplace-06-ci-module-signing/` artifacts updated to match. -- **Pre-commit follow-ups**: `pre-commit-verify-modules.sh` fails closed on unexpected `sig_policy` output and - on `git diff --cached` errors; `pre-commit-quality-checks.sh` documents suppressed `contract-test-status` - output and deduplicates the contract-first script existence check; script tests use a fake `hatch`, tighter - timeouts, and cover the no-module-tree skip path. --- diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index 82d9f649..bc2bbab9 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -199,10 +199,20 @@ run_module_signature_verification() { run_format_safety() { info "📦 Block 1 — format — running \`hatch run format\` (fails if working tree would change)" - local before_unstaged after_unstaged - before_unstaged=$(git diff --binary -- . || true) + local before_unstaged after_unstaged before_ec after_ec + before_ec=0 + before_unstaged=$(git diff --binary -- . 2>&1) || before_ec=$? + if [[ "${before_ec}" -gt 1 ]]; then + error "❌ git diff failed (cannot snapshot working tree before format; exit ${before_ec})" + exit 1 + fi if hatch run format; then - after_unstaged=$(git diff --binary -- . || true) + after_ec=0 + after_unstaged=$(git diff --binary -- . 2>&1) || after_ec=$? + if [[ "${after_ec}" -gt 1 ]]; then + error "❌ git diff failed (cannot snapshot working tree after format; exit ${after_ec})" + exit 1 + fi if [ "${before_unstaged}" != "${after_unstaged}" ]; then error "❌ Formatter changed files. Review and re-stage before committing." warn "💡 Run: hatch run format && git add -A" diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py index 12368a9d..d6d62125 100644 --- a/tests/unit/scripts/test_pre_commit_verify_modules.py +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -3,6 +3,7 @@ from __future__ import annotations import os +import shutil import stat import subprocess from pathlib import Path @@ -11,6 +12,7 @@ REPO_ROOT = Path(__file__).resolve().parents[3] +REAL_GIT = shutil.which("git") FLAG_SCRIPT = REPO_ROOT / "scripts" / "git-branch-module-signature-flag.sh" VERIFY_WRAPPER = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" @@ -64,6 +66,21 @@ def _write_fake_hatch(bin_dir: Path, log_path: Path) -> Path: return hatch +def _write_fake_git_fail_diff_cached(bin_dir: Path, real_git: str) -> Path: + git_bin = bin_dir / "git" + git_bin.write_text( + f"""#!/bin/sh +if [ "$1" = "diff" ] && [ "$2" = "--cached" ]; then + exit 2 +fi +exec "{real_git}" "$@" +""", + encoding="utf-8", + ) + git_bin.chmod(git_bin.stat().st_mode | stat.S_IXUSR | stat.S_IXGRP | stat.S_IXOTH) + return git_bin + + def _repo_with_verify_scripts( tmp_path: Path, *, @@ -103,10 +120,11 @@ def _repo_with_verify_scripts( if stage_module_paths: subprocess.run(["git", "add", "modules/pkg.yaml"], cwd=repo, check=True, capture_output=True, text=True) else: - (repo / "README.md").write_text("seed\n", encoding="utf-8") - subprocess.run(["git", "add", "README.md"], cwd=repo, check=True, capture_output=True, text=True) + docs = repo / "docs" + docs.mkdir(parents=True) + (docs / "notes.txt").write_text("seed\n", encoding="utf-8") subprocess.run( - ["git", "commit", "-m", "init"], + ["git", "add", "docs/notes.txt"], cwd=repo, check=True, capture_output=True, @@ -138,6 +156,29 @@ def test_verify_wrapper_skips_when_no_module_paths_staged(tmp_path: Path) -> Non assert log.strip() == "", "fake hatch must not run when module tree paths are not staged" +def test_verify_wrapper_propagates_git_diff_cached_failure(tmp_path: Path) -> None: + assert REAL_GIT is not None + repo, log_path = _repo_with_verify_scripts(tmp_path, stage_module_paths=True) + subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) + bin_dir = tmp_path / "bin" + bin_dir.mkdir(exist_ok=True) + _write_fake_hatch(bin_dir, log_path) + _write_fake_git_fail_diff_cached(bin_dir, REAL_GIT) + env = {**os.environ, "PATH": f"{bin_dir}:{os.environ.get('PATH', '')}"} + result = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode != 0, (result.stdout, result.stderr) + log = log_path.read_text(encoding="utf-8") + assert TOKEN_VERIFY_SCRIPT not in log + assert "git diff --cached failed" in result.stderr + + def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path) -> None: repo, log_path = _repo_with_verify_scripts(tmp_path) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) From 184f42628621e230e247b131a5ddaa87d9d07e33 Mon Sep 17 00:00:00 2001 From: Dominikus Nold Date: Tue, 14 Apr 2026 21:29:00 +0200 Subject: [PATCH 8/8] fix(pre-commit): legacy verify shim, mdc markdown, safe-change parity - Add pre-commit-verify-modules-signature.sh delegating to canonical verify - run_module_signature_verification: prefer canonical, fallback legacy, log path - Treat staged *.mdc like *.md; replace mapfile for Bash 3.2; drop pyproject/setup from Block 2 safe-change skip; extend tests for bundled module tree + legacy - Split pre-commit layout assertions to satisfy code-review complexity gate Made-with: Cursor --- CHANGELOG.md | 6 ++ scripts/pre-commit-quality-checks.sh | 47 +++++++++--- .../pre-commit-verify-modules-signature.sh | 6 ++ .../test_pre_commit_smart_checks_docs.py | 8 ++ .../scripts/test_pre_commit_verify_modules.py | 75 ++++++++++++++++--- .../test_trustworthy_green_checks.py | 43 +++++++---- 6 files changed, 148 insertions(+), 37 deletions(-) create mode 100755 scripts/pre-commit-verify-modules-signature.sh diff --git a/CHANGELOG.md b/CHANGELOG.md index c850fe8d..3dbeb7ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ All notable changes to this project will be documented in this file. deduplicates the contract-first script existence check, and treats `git diff` exit codes greater than 1 as errors in `run_format_safety` (exit 1 means “has diff”, not failure); script tests use a fake `hatch`, tighter timeouts, skip-path and `git diff --cached` failure coverage. +- **Legacy module verify path**: `scripts/pre-commit-verify-modules-signature.sh` is a small delegating shim to + `pre-commit-verify-modules.sh` for downstream hooks and mirrors; `run_module_signature_verification` prefers the + canonical script and falls back to the legacy path when only that file exists. +- **Pre-commit quality script**: staged Markdown detection includes `*.mdc`; Block 2 “safe change” no longer skips + review or contract tests for `pyproject.toml` / `setup.py` alone; markdown file lists avoid Bash 4 `mapfile` for + macOS Bash 3.2 compatibility. ### Changed diff --git a/scripts/pre-commit-quality-checks.sh b/scripts/pre-commit-quality-checks.sh index bc2bbab9..8eb29bea 100755 --- a/scripts/pre-commit-quality-checks.sh +++ b/scripts/pre-commit-quality-checks.sh @@ -73,7 +73,7 @@ has_staged_markdown() { local line while IFS= read -r line || [[ -n "${line}" ]]; do [[ -z "${line}" ]] && continue - if [[ "${line}" =~ \.md$ ]]; then + if [[ "${line}" =~ \.(md|mdc)$ ]]; then return 0 fi done < <(staged_files) @@ -95,7 +95,7 @@ staged_markdown_files() { local line while IFS= read -r line || [[ -n "${line}" ]]; do [[ -z "${line}" ]] && continue - if [[ "${line}" =~ \.md$ ]]; then + if [[ "${line}" =~ \.(md|mdc)$ ]]; then printf '%s\n' "${line}" fi done < <(staged_files) @@ -135,10 +135,10 @@ check_safe_change() { [[ -z "${file}" ]] && continue saw_any=true case "${file}" in - pyproject.toml|setup.py|src/__init__.py|src/specfact_cli/__init__.py) ;; + src/__init__.py|src/specfact_cli/__init__.py) ;; CHANGELOG.md|README.md|.pre-commit-config.yaml) ;; tools/smart_test_coverage.py|tools/functional_coverage_analyzer.py) ;; - *.md|*.rst|*.txt|*.json|*.yaml|*.yml) ;; + *.md|*.mdc|*.rst|*.txt|*.json|*.yaml|*.yml) ;; docs/*|papers/*|presentations/*|images/*) ;; .github/workflows/*) ;; *) @@ -182,16 +182,30 @@ run_version_sources_check_if_needed() { } run_module_signature_verification() { - local root + local root primary legacy chosen rel root=$(git rev-parse --show-toplevel 2>/dev/null || true) if [ -z "${root}" ]; then error "❌ Cannot resolve git repository root for module signature verification" exit 1 fi - if bash "${root}/scripts/pre-commit-verify-modules.sh"; then + primary="${root}/scripts/pre-commit-verify-modules.sh" + legacy="${root}/scripts/pre-commit-verify-modules-signature.sh" + chosen="" + if [[ -f "${primary}" ]]; then + chosen="${primary}" + rel="scripts/pre-commit-verify-modules.sh" + elif [[ -f "${legacy}" ]]; then + chosen="${legacy}" + rel="scripts/pre-commit-verify-modules-signature.sh (legacy entrypoint)" + else + error "❌ Missing module verify script: ${primary} and ${legacy} not found" + exit 1 + fi + info "📦 Module verify — running ${rel}" + if bash "${chosen}"; then success "✅ Module signature/version verification passed (or skipped — no staged module tree changes)" else - error "❌ Module signature/version verification failed" + error "❌ Module signature/version verification failed (${rel})" warn "💡 On main use --require-signature; elsewhere CI signs after PR approval" exit 1 fi @@ -241,12 +255,16 @@ run_yaml_lint_if_needed() { run_markdown_autofix_if_needed() { if ! has_staged_markdown; then - info "📦 Block 1 — Markdown fix — skipped (no staged *.md)" + info "📦 Block 1 — Markdown fix — skipped (no staged *.md / *.mdc)" return fi info "📦 Block 1 — Markdown fix — attempting safe auto-fix" local md_files=() - mapfile -t md_files < <(staged_markdown_files) + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + md_files+=("${line}") + done < <(staged_markdown_files) if ((${#md_files[@]} == 0)); then info "ℹ️ No staged markdown files resolved — skipping markdown auto-fix" return @@ -274,12 +292,16 @@ run_markdown_autofix_if_needed() { run_markdown_lint_if_needed() { if ! has_staged_markdown; then - info "📦 Block 1 — Markdown lint — skipped (no staged *.md)" + info "📦 Block 1 — Markdown lint — skipped (no staged *.md / *.mdc)" return fi info "📦 Block 1 — Markdown lint — running markdownlint" local md_files=() - mapfile -t md_files < <(staged_markdown_files) + local line + while IFS= read -r line || [[ -n "${line}" ]]; do + [[ -z "${line}" ]] && continue + md_files+=("${line}") + done < <(staged_markdown_files) if ((${#md_files[@]} == 0)); then return fi @@ -412,7 +434,7 @@ run_block2() { warn "🔍 specfact-cli pre-commit — Block 2 — hook: review + contract tests" if check_safe_change; then success "✅ Safe change detected — skipping Block 2 (code review + contract tests)" - info "💡 Only docs, workflow, version metadata, or allowlisted infra changed" + info "💡 Only docs (incl. *.mdc), workflow, version files, or allowlisted infra changed" exit 0 fi print_block2_overview @@ -435,6 +457,7 @@ run_all() { success "✅ Block 1 complete (all stages passed or skipped as expected)" if check_safe_change; then success "✅ Safe change detected — skipping Block 2 (code review + contract tests)" + info "💡 Only docs (incl. *.mdc), workflow, version files, or allowlisted infra changed" exit 0 fi print_block2_overview diff --git a/scripts/pre-commit-verify-modules-signature.sh b/scripts/pre-commit-verify-modules-signature.sh new file mode 100755 index 00000000..8adb08e9 --- /dev/null +++ b/scripts/pre-commit-verify-modules-signature.sh @@ -0,0 +1,6 @@ +#!/usr/bin/env bash +# Legacy entry point for module verify (pre-commit / downstream mirrors). +# Canonical script: pre-commit-verify-modules.sh (branch-aware marketplace-06 policy). +set -euo pipefail +_script_dir=$(cd "$(dirname "${BASH_SOURCE[0]:-$0}")" && pwd) +exec bash "${_script_dir}/pre-commit-verify-modules.sh" "$@" diff --git a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py index a7d1fc4c..e3f7eb55 100644 --- a/tests/unit/scripts/test_pre_commit_smart_checks_docs.py +++ b/tests/unit/scripts/test_pre_commit_smart_checks_docs.py @@ -57,3 +57,11 @@ def test_pre_commit_smart_checks_shim_delegates_to_quality_all() -> None: assert 'all "$@"' in shim assert "rev-parse --show-toplevel" in shim assert 'exec bash "${_repo_root}/scripts/pre-commit-quality-checks.sh"' in shim + + +def test_pre_commit_quality_markdown_globs_include_mdc() -> None: + script = _quality_script_text() + assert r"\.(md|mdc)$" in script + assert "mapfile" not in script + assert "pyproject.toml|setup.py|src/__init__.py" not in script + assert "*.md|*.mdc|*.rst" in script diff --git a/tests/unit/scripts/test_pre_commit_verify_modules.py b/tests/unit/scripts/test_pre_commit_verify_modules.py index d6d62125..7bf9bac2 100644 --- a/tests/unit/scripts/test_pre_commit_verify_modules.py +++ b/tests/unit/scripts/test_pre_commit_verify_modules.py @@ -15,6 +15,7 @@ REAL_GIT = shutil.which("git") FLAG_SCRIPT = REPO_ROOT / "scripts" / "git-branch-module-signature-flag.sh" VERIFY_WRAPPER = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" +LEGACY_VERIFY_WRAPPER = REPO_ROOT / "scripts" / "pre-commit-verify-modules-signature.sh" TOKEN_VERIFY_SCRIPT = "verify-modules-signature.py" TOKEN_REQUIRE_SIGNATURE = "--require-signature" @@ -86,15 +87,16 @@ def _repo_with_verify_scripts( *, flag_script_body: str | None = None, stage_module_paths: bool = True, + module_tree: str = "top", ) -> tuple[Path, Path]: - """Minimal git repo with verify/flag scripts; optionally stage paths under modules/.""" + """Minimal git repo with verify/flag scripts; optionally stage under modules/ or bundled tree.""" + assert module_tree in {"top", "bundled"} repo = tmp_path / "repo" scripts = repo / "scripts" scripts.mkdir(parents=True) - (repo / "modules").mkdir() - (repo / "modules" / "pkg.yaml").write_text("x: 1\n", encoding="utf-8") (scripts / "pre-commit-verify-modules.sh").symlink_to(VERIFY_WRAPPER.resolve()) + (scripts / "pre-commit-verify-modules-signature.sh").symlink_to(LEGACY_VERIFY_WRAPPER.resolve()) flag_target = scripts / "git-branch-module-signature-flag.sh" if flag_script_body is None: flag_target.symlink_to(FLAG_SCRIPT.resolve()) @@ -118,7 +120,16 @@ def _repo_with_verify_scripts( text=True, ) if stage_module_paths: - subprocess.run(["git", "add", "modules/pkg.yaml"], cwd=repo, check=True, capture_output=True, text=True) + if module_tree == "top": + mod_dir = repo / "modules" + mod_dir.mkdir(parents=True) + stage_path = "modules/pkg.yaml" + else: + mod_dir = repo / "src" / "specfact_cli" / "modules" + mod_dir.mkdir(parents=True) + stage_path = "src/specfact_cli/modules/pkg.yaml" + (mod_dir / "pkg.yaml").write_text("x: 1\n", encoding="utf-8") + subprocess.run(["git", "add", stage_path], cwd=repo, check=True, capture_output=True, text=True) else: docs = repo / "docs" docs.mkdir(parents=True) @@ -156,9 +167,46 @@ def test_verify_wrapper_skips_when_no_module_paths_staged(tmp_path: Path) -> Non assert log.strip() == "", "fake hatch must not run when module tree paths are not staged" -def test_verify_wrapper_propagates_git_diff_cached_failure(tmp_path: Path) -> None: +def test_pre_commit_verify_modules_legacy_entrypoint() -> None: + assert LEGACY_VERIFY_WRAPPER.is_file() + body = LEGACY_VERIFY_WRAPPER.read_text(encoding="utf-8") + assert "pre-commit-verify-modules.sh" in body + assert "exec bash" in body + + +@pytest.mark.parametrize("module_tree", ("top", "bundled")) +def test_legacy_verify_script_matches_canonical_invocation(tmp_path: Path, module_tree: str) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path, module_tree=module_tree) + subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) + env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} + canon = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + log_canon = log_path.read_text(encoding="utf-8") + log_path.write_text("", encoding="utf-8") + legacy = subprocess.run( + ["bash", str(repo / "scripts" / "pre-commit-verify-modules-signature.sh")], + cwd=repo, + env=env, + capture_output=True, + text=True, + timeout=30, + ) + log_legacy = log_path.read_text(encoding="utf-8") + assert canon.returncode == legacy.returncode == 0, (canon.stderr, legacy.stderr) + assert log_canon == log_legacy + assert TOKEN_VERIFY_SCRIPT in log_legacy + + +@pytest.mark.parametrize("module_tree", ("top", "bundled")) +def test_verify_wrapper_propagates_git_diff_cached_failure(tmp_path: Path, module_tree: str) -> None: assert REAL_GIT is not None - repo, log_path = _repo_with_verify_scripts(tmp_path, stage_module_paths=True) + repo, log_path = _repo_with_verify_scripts(tmp_path, stage_module_paths=True, module_tree=module_tree) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) bin_dir = tmp_path / "bin" bin_dir.mkdir(exist_ok=True) @@ -179,8 +227,9 @@ def test_verify_wrapper_propagates_git_diff_cached_failure(tmp_path: Path) -> No assert "git diff --cached failed" in result.stderr -def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path) -> None: - repo, log_path = _repo_with_verify_scripts(tmp_path) +@pytest.mark.parametrize("module_tree", ("top", "bundled")) +def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path, module_tree: str) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path, module_tree=module_tree) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} result = subprocess.run( @@ -199,8 +248,9 @@ def test_verify_wrapper_runs_hatch_with_require_on_main(tmp_path: Path) -> None: assert TOKEN_REQUIRE_SIGNATURE in log -def test_verify_wrapper_runs_hatch_checksum_only_off_main(tmp_path: Path) -> None: - repo, log_path = _repo_with_verify_scripts(tmp_path) +@pytest.mark.parametrize("module_tree", ("top", "bundled")) +def test_verify_wrapper_runs_hatch_checksum_only_off_main(tmp_path: Path, module_tree: str) -> None: + repo, log_path = _repo_with_verify_scripts(tmp_path, module_tree=module_tree) subprocess.run(["git", "branch", "-M", "feature/x"], cwd=repo, check=True, capture_output=True, text=True) env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} result = subprocess.run( @@ -219,9 +269,10 @@ def test_verify_wrapper_runs_hatch_checksum_only_off_main(tmp_path: Path) -> Non assert TOKEN_REQUIRE_SIGNATURE not in log -def test_verify_wrapper_rejects_invalid_sig_policy(tmp_path: Path) -> None: +@pytest.mark.parametrize("module_tree", ("top", "bundled")) +def test_verify_wrapper_rejects_invalid_sig_policy(tmp_path: Path, module_tree: str) -> None: bad_flag = "#!/usr/bin/env bash\nset -euo pipefail\necho bogus\n" - repo, _log_path = _repo_with_verify_scripts(tmp_path, flag_script_body=bad_flag) + repo, _log_path = _repo_with_verify_scripts(tmp_path, flag_script_body=bad_flag, module_tree=module_tree) subprocess.run(["git", "branch", "-M", "main"], cwd=repo, check=True, capture_output=True, text=True) env = {**os.environ, "PATH": f"{tmp_path / 'bin'}:{os.environ.get('PATH', '')}"} result = subprocess.run( diff --git a/tests/unit/workflows/test_trustworthy_green_checks.py b/tests/unit/workflows/test_trustworthy_green_checks.py index 15f7b29f..2d9ff915 100644 --- a/tests/unit/workflows/test_trustworthy_green_checks.py +++ b/tests/unit/workflows/test_trustworthy_green_checks.py @@ -165,12 +165,7 @@ def test_module_signature_check_name_is_canonical_across_workflows() -> None: assert orchestrator_name == dedicated_name == "Verify Module Signatures" -def test_pre_commit_config_matches_modular_quality_layout() -> None: - """Local hooks should mirror specfact-cli-modules: fail_fast, verify, block1 stages, block2.""" - config = _load_yaml(PRE_COMMIT_CONFIG) - assert config.get("fail_fast") is True - hooks = _load_hooks() - by_id = {h.get("id"): h for h in hooks if isinstance(h.get("id"), str)} +def _assert_pre_commit_verify_and_version_hooks(by_id: dict[str, dict[str, Any]]) -> None: assert "verify-module-signatures" in by_id verify_hook = by_id["verify-module-signatures"] assert verify_hook.get("always_run") is True @@ -178,15 +173,23 @@ def test_pre_commit_config_matches_modular_quality_layout() -> None: assert "pre-commit-verify-modules.sh" in str(verify_hook.get("entry", "")) verify_script = REPO_ROOT / "scripts" / "pre-commit-verify-modules.sh" assert verify_script.is_file() + legacy_verify = REPO_ROOT / "scripts" / "pre-commit-verify-modules-signature.sh" + assert legacy_verify.is_file() assert "--payload-from-filesystem" in verify_script.read_text(encoding="utf-8") assert "check-version-sources" in by_id - assert "cli-block1-format" in by_id - assert "cli-block1-yaml" in by_id - assert "cli-block1-markdown-fix" in by_id - assert "cli-block1-markdown-lint" in by_id - assert "cli-block1-workflows" in by_id - assert "cli-block1-lint" in by_id - assert "cli-block2" in by_id + + +def _assert_pre_commit_cli_quality_block_hooks(by_id: dict[str, dict[str, Any]]) -> None: + for hid in ( + "cli-block1-format", + "cli-block1-yaml", + "cli-block1-markdown-fix", + "cli-block1-markdown-lint", + "cli-block1-workflows", + "cli-block1-lint", + "cli-block2", + ): + assert hid in by_id assert by_id["cli-block2"].get("always_run") is True for hid in ( "cli-block1-format", @@ -202,6 +205,20 @@ def test_pre_commit_config_matches_modular_quality_layout() -> None: assert "check-doc-frontmatter" in by_id +def test_pre_commit_config_matches_modular_quality_layout() -> None: + """Local hooks should mirror specfact-cli-modules: fail_fast, verify, block1 stages, block2.""" + config = _load_yaml(PRE_COMMIT_CONFIG) + assert config.get("fail_fast") is True + hooks = _load_hooks() + by_id: dict[str, dict[str, Any]] = {} + for h in hooks: + hid = h.get("id") + if isinstance(hid, str): + by_id[hid] = h + _assert_pre_commit_verify_and_version_hooks(by_id) + _assert_pre_commit_cli_quality_block_hooks(by_id) + + def test_coderabbit_auto_review_covers_dev_and_main() -> None: """Automatic review coverage should be consistent for both protected integration branches.""" config = _load_yaml(CODERABBIT_CONFIG)