diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9c5d46d..4445f7c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,7 +14,7 @@ concurrency: jobs: validate: - name: Lint and validate configuration + name: Lint and test runs-on: ubuntu-latest steps: - name: Check out repository @@ -36,12 +36,6 @@ jobs: - name: Check formatting run: uv run ruff format --check . - # Catches the failure modes that break the template silently for whoever - # copies it: unparseable config, hooks pointing at deleted scripts, - # dropped CLAUDE.md rule imports, skills Claude can no longer discover. - - name: Validate Claude Code configuration - run: uv run python scripts/validate_config.py - # The hooks are the enforced guardrails, and shell quoting bugs in them # have shipped before. Warning severity skips style nitpicks. - name: Shellcheck hooks diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d637ea..cec3a6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,9 +8,8 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added -- **Tests** (`tests/`): 65 tests covering the six hook scripts. Each runs the real script in a subprocess against a crafted payload and asserts on the decision it emits — nothing is mocked, since "the hook silently stopped firing" is the regression worth catching and a mock cannot fail that way. Validated by mutation: reintroducing the pre-v1.3.0 `\b` boundary in `protect-main.sh` turns 8 tests red in both directions. `verify.sh`'s ruff/pytest body and `auto-lint.sh`'s formatting body are left uncovered on purpose -- **CI** (`.github/workflows/ci.yml`): GitHub Actions workflow on pull requests and pushes to `main`, running ruff (lint + format), `scripts/validate_config.py`, `shellcheck --severity=warning` over the hook scripts, and `pytest`. Nothing verified this repository before — every earlier pull request merged without a single automated check -- **Scripts** (`scripts/validate_config.py`): checks that `settings.json` and the MCP configs parse, that the hook scripts `settings.json` references exist, that `CLAUDE.md`'s `@`-imports resolve, and that every skill's `SKILL.md` declares `name` and `description`. All but the first fail silently at runtime. Verified by injecting each fault and confirming a non-zero exit +- **Tests** (`tests/`): 75 tests covering the six hook scripts. Each runs the real script in a subprocess against a crafted payload and asserts on the decision it emits — nothing is mocked, since "the hook silently stopped firing" is the regression worth catching and a mock cannot fail that way. Validated by mutation: reintroducing the pre-v1.3.0 `\b` boundary in `protect-main.sh` turns 8 tests red in both directions. `verify.sh`'s ruff/pytest body and `auto-lint.sh`'s formatting body are left uncovered on purpose +- **CI** (`.github/workflows/ci.yml`): GitHub Actions workflow on pull requests and pushes to `main`, running ruff (lint + format), `shellcheck --severity=warning` over the hook scripts, and `pytest`. Nothing verified this repository before — every earlier pull request merged without a single automated check - **README.md**: a "Bash Sandbox" section, with a paste-ready `sandbox` block for a uv-based project. Deliberately not enabled in `.claude/settings.json`: the sandbox does not run on native Windows, and enabling it in checked-in project settings would warn at startup for every Windows contributor - **README.md**: `bubblewrap` and `socat` added to the optional dependencies table, needed by the sandbox on Linux and WSL2 - **.env.example** / **README.md**: six Claude Code tuning variables — `BASH_DEFAULT_TIMEOUT_MS`, `BASH_MAX_OUTPUT_LENGTH`, `CLAUDE_CODE_MAX_SUBAGENT_SPAWN_DEPTH`, `CLAUDE_CODE_MAX_CONCURRENT_SUBAGENTS`, `CLAUDE_CODE_MAX_WEB_SEARCHES_PER_SESSION` and `CLAUDE_CODE_MCP_AUTO_BACKGROUND_MS`. The first two are the ones a pytest-based project hits in practice — a suite over two minutes gets killed mid-run, and verbose output truncates before the failure summary; the rest are recorded at their defaults @@ -22,10 +21,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - **`docx`, `pptx`, `xlsx`** picked up upstream's 2026-07-17 consolidation of the shared `scripts/office/` helpers, which carries security fixes, plus `.dotx`/`.potx`/`.xltx` template support - **`frontend-design`** was rewritten upstream from "production-grade interfaces" into visual-design direction - The remaining skills changed only in `LICENSE.txt`, or not at all -- **pyproject.toml**: ruff's `include` was `["pyproject.toml", "src/**/*.py"]`, which matched nothing this repository actually ships, since there is no `src/`. Extended to `scripts/**/*.py`, `tests/**/*.py` and `.claude/statusline.py`, with `tests` added to `known-first-party`. Vendored `.claude/skills/` scripts stay out of scope +- **pyproject.toml**: ruff's `include` was `["pyproject.toml", "src/**/*.py"]`, which matched nothing this repository actually ships, since there is no `src/`. Extended to `tests/**/*.py` and `.claude/statusline.py`, with `tests` added to `known-first-party`. Vendored `.claude/skills/` scripts stay out of scope - **README.md**: recorded which ten skills are vendored copies from upstream, that they do not self-update, and the sync point (`f17010c`, 2026-08-13) so the next refresh has a baseline. Refreshed the `claude-api`, `docx`, `pptx`, `skill-creator` and `frontend-design` rows to match their new upstream descriptions - **README.md**: rewrote the passages that narrated change instead of describing the current state — the fullscreen-rendering and background-subagent defaults, the permission-mode label, and the rationale for the hook tests. Change history belongs in this file, not in the README -- **CONTRIBUTING.md**: the contributor verification command ran ruff alone while CI runs four checks, so a contributor could be green locally and red in CI. Aligned both commands, noted that `shellcheck` runs in CI, and recorded that the hook tests need `bash` and `jq` on PATH (they skip without them) +- **CONTRIBUTING.md**: the contributor verification command ran ruff alone while CI also runs the hook tests, so a contributor could be green locally and red in CI. Aligned both commands, noted that `shellcheck` runs in CI, and recorded that the hook tests need `bash` and `jq` on PATH (they skip without them) - **README.md**: replaced the "many more events" aside in the hooks section with the current count (32) and the events most useful for extending this setup - **README.md**: documented that `permissions.defaultMode: "default"` is labeled "Manual" in the interface, with `"manual"` accepted as an alias - **Settings** (`.claude/settings.json`): added `permissions.defaultMode: "default"` explicitly — same behavior as the implicit default, but now visible and easy to customize @@ -42,7 +41,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed -- **Hooks** (`enforce-uv.sh`, `protect-main.sh`): both matched their patterns against the raw command string, so text carried in a heredoc body was read as code. A `gh pr create --body "$(cat <<'EOF' … EOF)"` whose description quoted `ruff format` after an `&&`, or `pip install`, or `rm -rf /` at the start of a line, was denied — the hook blocking a command that never intended to run any of it. Found when `enforce-uv` refused the `gh pr create` for the documentation coherence pass. Both now match against the command with heredoc bodies stripped, via a shared `hooks/lib/command-text.sh`; rewrites still emit the original command. Code before or after a heredoc is unaffected. The deliberate trade is that a heredoc fed to an interpreter (`bash </lib/` in any project built from this template. Anchored both to the repository root, where distutils actually writes them - **Hooks** (`session-start.sh`): `input=$(cat)` assigned the payload to a variable the script never read (shellcheck `SC2034`), caught by the new CI on its first run. Not a bug — the hook inspects the filesystem, not the payload — but the dead assignment implied otherwise. Replaced with `cat >/dev/null` and a comment noting stdin is drained so Claude Code's write to the pipe completes - **Hooks** (`protect-main.sh`): the broad `rm -rf` guard used `\b` to close each dangerous target (`/`, `.`, `..`, `~`), which does not behave as a token boundary — it matches on any adjacent word character and never matches at end-of-string. The guard let through the most common forms (`rm -rf .`, `rm -rf ..`, `rm -rf /`, `rm -rf ~`) while incorrectly blocking legitimate targets like `rm -rf .git` and `rm -rf ~/tmp-dir`. Replaced with `($|\s)` so it matches the whole token diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index be5993c..f5ab9d9 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -22,9 +22,9 @@ Open a [feature request](https://github.com/skateddu/claude-code-python-setup/is 1. Fork the repository 2. Create a branch from `main`: `git checkout -b feature/your-change` 3. Make your changes -4. Run the verification checks — these are the same four the CI runs, so passing here means passing there: +4. Run the verification checks — the same ones CI runs, so passing here means passing there: ```bash - uv run ruff check . && uv run ruff format --check . && uv run python scripts/validate_config.py && uv run pytest + uv run ruff check . && uv run ruff format --check . && uv run pytest ``` CI additionally runs `shellcheck --severity=warning .claude/hooks/*.sh`, which needs [shellcheck](https://github.com/koalaman/shellcheck#installing) locally. 5. Commit using [Conventional Commits](https://www.conventionalcommits.org/) format: @@ -44,7 +44,7 @@ cd claude-code-python-setup uv sync # Verify everything works -uv run ruff check . && uv run ruff format --check . && uv run python scripts/validate_config.py && uv run pytest +uv run ruff check . && uv run ruff format --check . && uv run pytest ``` The hook tests shell out to the scripts in `.claude/hooks/`, so they need `bash` and `jq` on your PATH. Without them the suite skips rather than fails. diff --git a/README.md b/README.md index a7215b1..8cfd79b 100644 --- a/README.md +++ b/README.md @@ -149,8 +149,6 @@ claude-code-python-setup/ ├── mcp_config/ │ ├── linux_mac.mcp.json # MCP server config (Linux/Mac) │ └── windows.mcp.json # MCP server config (Windows) -├── scripts/ -│ └── validate_config.py # Checks this template's own config is coherent ├── tests/ │ ├── conftest.py # Fixtures that run a hook against a payload │ ├── hook_harness.py # HookResult and payload builders @@ -551,19 +549,18 @@ The script is written in Python and works cross-platform: Windows, macOS, and Li ## Continuous Integration -`.github/workflows/ci.yml` runs on every pull request and on pushes to `main`. It lints, checks that the configuration this template ships is internally coherent, and runs the hook tests — the kinds of breakage that would otherwise reach whoever copies the `.claude/` folder. +`.github/workflows/ci.yml` runs on every pull request and on pushes to `main`. It lints and runs the hook tests, so the guardrails this template ships are known to work before anyone copies the `.claude/` folder. | Check | Catches | |-------|---------| -| `ruff check` / `ruff format --check` | Lint and formatting on `.claude/statusline.py`, `scripts/` and `tests/` (`src/**/*.py` is in scope for projects built from this template) | -| `scripts/validate_config.py` | Unparseable `settings.json` or MCP config; hooks pointing at scripts that no longer exist; `CLAUDE.md` `@`-imports that don't resolve; skills whose `SKILL.md` lost its `name`/`description` frontmatter | +| `ruff check` / `ruff format --check` | Lint and formatting on `.claude/statusline.py` and `tests/` (`src/**/*.py` is in scope for projects built from this template) | | `shellcheck --severity=warning` | Shell quoting and syntax bugs in the hook scripts | | `pytest` | Hooks reaching the **wrong decision** — see below | -The `validate_config.py` failures are all silent at runtime: a hook whose script was renamed simply stops firing, and a broken `@`-import drops that rule from Claude's context without an error. Run everything locally with: +Run the same checks locally with: ```bash -uv run ruff check . && uv run ruff format --check . && uv run python scripts/validate_config.py && uv run pytest +uv run ruff check . && uv run ruff format --check . && uv run pytest ``` ### Hook tests (`tests/`) @@ -587,7 +584,7 @@ Two gaps are deliberate and worth knowing: - **`verify.sh`'s ruff/pytest execution path is not covered.** Running it from inside the suite would invoke pytest recursively, and a throwaway uv project would need a network install on every CI run. Only its guard clauses are tested. - **`auto-lint.sh`'s formatting path is not covered**, because whether ruff acts on a given file depends on the surrounding project's `include` configuration. Only the conditions under which the hook must do nothing are tested. -Neither the tests nor `validate_config.py` check the **hook wiring** in `settings.json` — that a hook's `matcher` and `if` condition actually route the events you expect. A hook can be correct, referenced, and still never fire. That wiring is verifiable only by running Claude Code. +The tests do not check the **hook wiring** in `settings.json` — that a hook's `matcher` and `if` condition actually route the events you expect. A hook can be correct, referenced, and still never fire. That wiring is verifiable only by running Claude Code. ## Contributing diff --git a/pyproject.toml b/pyproject.toml index bd794fa..cc3074c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -56,13 +56,7 @@ line-length = 99 src = ["src"] # src/** is for projects built from this template; the other two are the Python # this repo actually ships. Vendored .claude/skills/ scripts stay out of scope. -include = [ - "pyproject.toml", - "src/**/*.py", - "tests/**/*.py", - "scripts/**/*.py", - ".claude/statusline.py", -] +include = ["pyproject.toml", "src/**/*.py", "tests/**/*.py", ".claude/statusline.py"] [tool.ruff.lint] extend-select = ["I"] # Add import sorting diff --git a/scripts/validate_config.py b/scripts/validate_config.py deleted file mode 100644 index 4ae5f16..0000000 --- a/scripts/validate_config.py +++ /dev/null @@ -1,152 +0,0 @@ -"""Validate the Claude Code configuration this template ships.""" - -from __future__ import annotations - -import json -from pathlib import Path -import re -import sys - -REPO_ROOT = Path(__file__).resolve().parent.parent -SETTINGS_PATH = REPO_ROOT / ".claude" / "settings.json" -MCP_CONFIG_DIR = REPO_ROOT / "mcp_config" -CLAUDE_MD_PATH = REPO_ROOT / "CLAUDE.md" -SKILLS_DIR = REPO_ROOT / ".claude" / "skills" - -# Hook entries name their script inline, e.g. "bash .claude/hooks/verify.sh". -HOOK_SCRIPT_PATTERN = re.compile(r"\.claude/hooks/[\w.-]+\.sh") -# CLAUDE.md pulls in modular rules with "@.claude/rules/.md". -IMPORT_PATTERN = re.compile(r"@(\.claude/rules/[\w.-]+\.md)") -# Skills declare their identity in YAML frontmatter at the top of SKILL.md. -FRONTMATTER_PATTERN = re.compile(r"\A---\r?\n(.*?)\r?\n---", re.S) - - -def check_json_parses() -> list[str]: - """Report config files that are not valid JSON. - - Returns - ------- - list[str] - One message per unparseable file; empty when all parse. - """ - errors: list[str] = [] - for path in [SETTINGS_PATH, *sorted(MCP_CONFIG_DIR.glob("*.json"))]: - try: - json.loads(path.read_text(encoding="utf-8")) - except ValueError as error: - errors.append(f"{path.relative_to(REPO_ROOT)}: invalid JSON -> {error}") - return errors - - -def check_hook_scripts_exist() -> list[str]: - """Report hook scripts referenced by settings.json that are missing. - - A hook pointing at a deleted script fails silently at runtime, so the - guardrail is simply gone with no error surfaced to the user. - - Returns - ------- - list[str] - One message per dangling reference; empty when all resolve. - """ - settings_text = SETTINGS_PATH.read_text(encoding="utf-8") - referenced = sorted(set(HOOK_SCRIPT_PATTERN.findall(settings_text))) - if not referenced: - return ["`.claude/settings.json`: no hook scripts referenced — did the format change?"] - return [ - f".claude/settings.json references missing hook: {reference}" - for reference in referenced - if not (REPO_ROOT / reference).is_file() - ] - - -def check_claude_md_imports() -> list[str]: - """Report `@`-imports in CLAUDE.md that do not resolve to a file. - - A broken import drops that rule from Claude's context without warning. - - Returns - ------- - list[str] - One message per unresolved import; empty when all resolve. - """ - imports = sorted(set(IMPORT_PATTERN.findall(CLAUDE_MD_PATH.read_text(encoding="utf-8")))) - if not imports: - return ["CLAUDE.md: no @-imports found — did the rules section move?"] - return [ - f"CLAUDE.md imports missing file: @{target}" - for target in imports - if not (REPO_ROOT / target).is_file() - ] - - -def check_skill_frontmatter() -> list[str]: - """Report skills whose SKILL.md lacks usable frontmatter. - - Claude discovers a skill through the `name` and `description` fields, so a - skill missing either is invisible to automatic invocation. - - Returns - ------- - list[str] - One message per malformed skill; empty when all are well formed. - """ - errors: list[str] = [] - for skill_dir in sorted(path for path in SKILLS_DIR.iterdir() if path.is_dir()): - skill_file = skill_dir / "SKILL.md" - if not skill_file.is_file(): - errors.append(f"{skill_dir.name}: no SKILL.md") - continue - - match = FRONTMATTER_PATTERN.match(skill_file.read_text(encoding="utf-8")) - if match is None: - errors.append(f"{skill_dir.name}/SKILL.md: no YAML frontmatter block") - continue - - frontmatter = match.group(1) - missing = [ - field - for field in ("name", "description") - if not re.search(rf"^{field}:", frontmatter, re.M) - ] - if missing: - errors.append(f"{skill_dir.name}/SKILL.md: frontmatter missing {', '.join(missing)}") - return errors - - -def main() -> int: - """Run every check and report the outcome. - - Returns - ------- - int - Process exit code: 0 when every check passes, 1 otherwise. - """ - checks = { - "JSON config parses": check_json_parses, - "hook scripts exist": check_hook_scripts_exist, - "CLAUDE.md imports resolve": check_claude_md_imports, - "skill frontmatter is complete": check_skill_frontmatter, - } - - failures = 0 - for label, check in checks.items(): - errors = check() - if errors: - failures += len(errors) - print(f"FAIL {label}") - for error in errors: - print(f" {error}") - else: - print(f"ok {label}") - - if failures: - print(f"\n{failures} problem(s) found.") - return 1 - - print("\nAll configuration checks passed.") - return 0 - - -if __name__ == "__main__": - sys.exit(main())