diff --git a/.claude-plugin/plugin.json b/.claude-plugin/plugin.json index b14009c..92840e0 100644 --- a/.claude-plugin/plugin.json +++ b/.claude-plugin/plugin.json @@ -1,6 +1,6 @@ { "name": "forge", - "version": "1.23.2", + "version": "1.24.0", "description": "Automate Python CI/CD and code-quality standards — deterministic CLIs + a drop-in pre-commit hook, runnable with or without an AI agent. This optional Claude Code plugin orchestrates them; the gate is the CLI, never the model.", "author": { "name": "Jean Simonnet", diff --git a/CHANGELOG.md b/CHANGELOG.md index 9af9816..40c5aa8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,29 @@ change groups by conventional-commit type (**Features / Fixes / Refactor Follows [Keep a Changelog](https://keepachangelog.com/) in spirit; versions follow forge's rolling-next convention. +## v1.24.0 — 2026-06-17 + +All additive and opt-in — no consumer action required to upgrade. + +### Features +- **Pluggable pre-commit step framework** — `[tool.forge.precommit] + enable` / `disable` (plus `forge-precommit --only` / `--skip`) turn any + step on or off uniformly, on top of each step's own self-skip (#6). +- **Opt-in `doctest` step** — `pytest --doctest-modules` over + `[tool.forge.doctest].paths` (default `["src"]`); non-blocking by + default (#5). +- **Opt-in `typecheck` step** — runs `pyrefly` over + `[tool.forge.typecheck].paths`; non-blocking by default (#48). +- **Opt-in `doc_consistency` step** + `verify-forge-doc-consistency` CLI — + checks that every `[project.scripts]` CLI is documented in + `docs/cli-reference.md`; non-blocking (#4). + +### Tooling +- `forge-config --list` now enumerates the new + `[tool.forge.precommit/doctest/typecheck]` keys, and a drift test + couples `CONFIG_KEYS` to its readers so the registry can't silently go + stale (#46). + ## v1.23.0 — 2026-06-17 ### Features diff --git a/CLAUDE.md b/CLAUDE.md index ecaeaa0..277ea8e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -13,6 +13,7 @@ ## Forge-specific rules - **Version derivation**: pip package version comes from the latest git tag via setuptools-scm. There is no manual `version = "x.y.z"` in `pyproject.toml`. Release flow: `git tag vX.Y.Z && git push origin vX.Y.Z`. +- **PR & branch granularity — bundle cohesive work, don't reflex-split.** A PR is the smallest *cohesive* unit, not the smallest possible one. In forge every PR also costs a rolling-next `plugin.json` bump + a `dev` tag (and, at minor boundaries, a promotion), so splitting *related* changes across PRs multiplies that ceremony to buy isolation nobody needs. **Default to ONE PR** when the changes share a subsystem, must land together, or one half is meaningless without the other — e.g. a bug a type checker surfaces *while you build the feature that runs it* belongs in that feature's PR, not a parallel one; a follow-up cleanup to code this PR introduces belongs here too. **Split only for a concrete reason:** independent risk/rollback, different reviewers or release cadence, a hard blocking dependency, or a diff too large to review in one sitting. "It's a separate concern" does **not** justify a separate PR when the concerns ship together — separateness of *topic* ≠ separateness of *delivery*. When unsure, bundle. (File a separate *issue* freely for deferred work — that's cheap; it's separate *PRs/branches* that add the burden.) - **Release process — versioning, tag cadence, and `dev → main` promotion** are specified in **[`docs/release-process.md`](docs/release-process.md)**, the single source of truth + the **invariant→test contract**. Do not restate its mechanics elsewhere; point here. Operational summary: - `.claude-plugin/plugin.json["version"]` is **rolling-next** — always names the version about to be released; `step_plugin_version` enforces `plugin.json > latest_tag` on every commit (skipped when `HEAD` reproduces a tagged release's tree). After tagging `vX.Y.Z`, the next PR must bump it. - Tag `dev` `vX.Y.Z` after **every** merge to dev (`forge-next-prep --tag`, run by `/next` — **never drop `--tag`**); tag `main` **only** at the minor-boundary promotion. `@dev` gets every version, `@main` minors only. diff --git a/REPO_STRUCTURE.md b/REPO_STRUCTURE.md index f714aa8..dc30f6e 100644 --- a/REPO_STRUCTURE.md +++ b/REPO_STRUCTURE.md @@ -37,6 +37,7 @@ Code. structure drift check - verify_test_naming.py: `verify-forge-test-naming` — test naming check - verify_manifest.py: `verify-forge-manifest` — `.claude-plugin/*.json` JSON validation + - verify_doc_consistency.py: `verify-forge-doc-consistency` — checks every `[project.scripts]` CLI is documented in `docs/cli-reference.md`; backs the opt-in `doc_consistency` pre-commit step (non-blocking) - verify_plugin_version.py: `verify-forge-plugin-version` — rolling-next guard (plugin.json["version"] > latest git tag) - gen_cli_reference.py: `forge-gen-cli-reference` — CLI reference doc generator @@ -159,6 +160,7 @@ Pytest suite mirroring the `src/forge/` layout: - test_verify_docstrings.py: tests for verify_docstrings - test_verify_docstring_coverage.py: tests for verify_docstring_coverage - test_verify_manifest.py: tests for verify_manifest + - test_verify_doc_consistency.py: tests for verify_doc_consistency - test_verify_plugin_version.py: tests for verify_plugin_version - test_verify_repo_structure.py: tests for verify_repo_structure - test_verify_test_naming.py: tests for verify_test_naming diff --git a/dev/setup.sh b/dev/setup.sh index 1294c7b..2b1c9ef 100755 --- a/dev/setup.sh +++ b/dev/setup.sh @@ -65,9 +65,12 @@ if [ "${CONDA_DEFAULT_ENV:-}" != "$ENV_NAME" ]; then exit 1 fi -# 4. Editable install with test deps. -echo -e "${YELLOW}→${NC} pip install -e .[test]" -pip install -e ".[test]" +# 4. Editable install with the full contributor umbrella (test + audit + +# typecheck). forge's own dev env should carry every tool it ships and +# dogfoods — pytest, the audit deps (vulture / jsonschema / PyYAML), and +# pyrefly for the opt-in typecheck step. `[dev]` is that umbrella. +echo -e "${YELLOW}→${NC} pip install -e .[dev]" +pip install -e ".[dev]" # 5. Run the consumer-facing umbrella installer. Dogfood: forge is the # source of install-forge-bootstrap, so running it here exercises the diff --git a/docs/api-digest.md b/docs/api-digest.md index 74adb2e..2547ab9 100644 --- a/docs/api-digest.md +++ b/docs/api-digest.md @@ -4,7 +4,7 @@ A compact index of this codebase's symbols — every top-level function and clas > **Generated file — do not edit by hand.** Regenerate with `forge-gen-api-digest`; check for drift with `forge-gen-api-digest --check`. -_40 modules, 362 symbols._ +_41 modules, 374 symbols._ ## `forge._hook_helpers` @@ -158,7 +158,7 @@ _40 modules, 362 symbols._ ## `forge.config` -- `class ForgeConfig` — Branch-name configuration sourced from ``[tool.forge]``. +- `class ForgeConfig` — Repo configuration sourced from ``[tool.forge]``. - `dual_track(self) -> bool` — Return ``True`` when base and dev are distinct branches. - `read_pyproject_raw(repo_root: Path) -> dict` — Return the full parsed ``pyproject.toml`` dict, or ``{}`` on failure. - `load_config(repo_root: Path) -> ForgeConfig` — Read ``[tool.forge]`` from *repo_root*'s ``pyproject.toml``. @@ -326,6 +326,7 @@ _40 modules, 362 symbols._ - `_read_plugin_version_at_ref(repo_root: Path, ref: str) -> str | None` _(internal)_ — Return ``plugin.json["version"]`` at the given git ref, or ``None`` when absent. - `_check_promote_pending_message(repo_root: Path, dev_branch: str, base_branch: str) -> str | None` _(internal)_ — Return a one-line user-facing prompt when promotion is pending, else ``None``. +- `_changelog_lacks_entry(changelog_text: str, minor_tag: str) -> bool` _(internal)_ — Return True when *changelog_text* has no ``## `` heading. - `_promotion_status_lines(repo_root: Path, dev_branch: str, base_branch: str) -> list[str]` _(internal)_ — Build the read-only promotion-status report. - `_git(*args: str, cwd: Path | None = None, check: bool = True) -> str` _(internal)_ — Run ``git`` with *args*, return stripped stdout. - `_read_plugin_version(repo_root: Path) -> str | None` _(internal)_ — Return ``.claude-plugin/plugin.json["version"]`` or ``None`` if absent. @@ -369,6 +370,8 @@ _40 modules, 362 symbols._ - `_color(code: str) -> str` _(internal)_ — Return *code* if stdout is a TTY, else an empty string. - `class StepResult` — Outcome of a single pre-commit step. +- `class StepDef` — A registry entry: a step's name, its function, and whether it runs by default. +- `_forge_step_config(repo_root: Path, step: str) -> dict[str, object]` _(internal)_ — Return the ``[tool.forge.]`` table, or ``{}`` when absent. - `_run(cmd: list[str], cwd: Path) -> tuple[bool, str]` _(internal)_ — Run *cmd* and capture combined output. - `step_ruff(repo_root: Path) -> StepResult` — Run ``fix-forge-ruff`` — owns the ruff phase end-to-end. - `step_docstrings(repo_root: Path) -> StepResult` — Run ``verify-forge-docstrings`` over the current diff vs main. @@ -382,9 +385,16 @@ _40 modules, 362 symbols._ - `step_cli_wiring(repo_root: Path) -> StepResult` — Run ``verify-forge-cli-wiring`` — assert every script has a real caller. - `_cli_wiring_enabled(repo_root: Path) -> bool` _(internal)_ — Return True when the repo has opted into the cli_wiring check. - `step_plugin_version(repo_root: Path) -> StepResult` — Run ``verify-forge-plugin-version`` — owns the rolling-next guard. +- `_bad_scan_paths(paths: list[object], repo_root: Path) -> list[str]` _(internal)_ — Return config scan-path values that are option-like or escape the repo. +- `step_doctest(repo_root: Path) -> StepResult` — Run ``pytest --doctest-modules`` over docstring examples (opt-in). +- `step_typecheck(repo_root: Path) -> StepResult` — Run pyrefly over the source tree (opt-in). +- `step_doc_consistency(repo_root: Path) -> StepResult` — Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). - `_write_log(repo_root: Path, result: StepResult) -> None` _(internal)_ — Persist *result*'s output to ``code_health/.log``. - `_print_step_line(result: StepResult) -> None` _(internal)_ — Print a one-line status for *result* (SKIP/PASS/WARN/FAIL). -- `run_all(repo_root: Path | None = None, *, print_progress: bool = True) -> list[StepResult]` — Run every step in order and return their results. +- `_validate_step_names(names: Sequence[str]) -> None` _(internal)_ — Raise ``ValueError`` listing any *names* that are not registered steps. +- `_resolve_steps(repo_root: Path, *, skip: Sequence[str] = (), only: Sequence[str] = ()) -> list[StepDef]` _(internal)_ — Resolve which steps to run, in registry order. +- `run_all(repo_root: Path | None = None, *, print_progress: bool = True, skip: Sequence[str] = (), only: Sequence[str] = ()) -> list[StepResult]` — Run the resolved step sequence in order and return their results. +- `_split_csv(values: Sequence[str]) -> list[str]` _(internal)_ — Flatten repeatable / comma-separated CLI values into a clean name list. - `main() -> int` — CLI entry point. ## `forge.run_context` @@ -432,6 +442,11 @@ _40 modules, 362 symbols._ - `_check_wiring(root: Path) -> int` _(internal)_ — Run the reachability check and report findings. - `main() -> int` — Entry point for ``verify-forge-cli-wiring``. +## `forge.verify_doc_consistency` + +- `_check_cli_coverage(repo_root: Path) -> list[str]` _(internal)_ — Return findings for ``[project.scripts]`` names missing from the CLI reference. +- `main() -> int` — CLI entry point. + ## `forge.verify_docstring_coverage` - `_interrogate_config(data: dict) -> tuple[InterrogateConfig, float, list[str]]` _(internal)_ — Build the interrogate config + threshold + excludes from TOML data. @@ -464,7 +479,7 @@ _40 modules, 362 symbols._ ## `forge.verify_plugin_version` -- `_is_release_commit(repo_root: Path, tag: str) -> bool` _(internal)_ — Return True when ``HEAD`` carries the same file content as *tag*. +- `_is_release_commit(repo_root: Path) -> bool` _(internal)_ — Return True when ``HEAD``'s tree reproduces ANY published ``v*`` tag. - `main() -> int` — Enforce plugin.json version > latest git tag. ## `forge.verify_repo_structure` diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 9bf1f36..a64f266 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -375,7 +375,8 @@ options: ## forge-precommit ```text -usage: forge-precommit [-h] [--json] +usage: forge-precommit [-h] [--json] [--skip STEP[,STEP...]] + [--only STEP[,STEP...]] Run the forge pre-commit check sequence: ruff (format + check, self-healing with --unsafe-fixes on failure) + docstring verification (diff vs main) + @@ -387,8 +388,14 @@ default sequence — run it in CI or wire it into .githooks/pre-commit explicitly. Used by any repo that adopts forge via install-forge-githooks. options: - -h, --help show this help message and exit - --json Emit a JSON summary on stdout instead of human output. + -h, --help show this help message and exit + --json Emit a JSON summary on stdout instead of human output. + --skip STEP[,STEP...] + Force-skip these steps for this run (repeatable or + comma-separated). + --only STEP[,STEP...] + Run exactly these steps (repeatable or comma- + separated). ``` ## forge-slow-tests-report @@ -527,6 +534,19 @@ options: -h, --help show this help message and exit ``` +## verify-forge-doc-consistency + +```text +usage: verify-forge-doc-consistency [-h] + +Check machine-checkable documentation claims (CLI name-lists, agent counts) +against the actual repo state. Non-blocking reporter for the doc_consistency +pre-commit step. + +options: + -h, --help show this help message and exit +``` + ## verify-forge-docstring-coverage ```text diff --git a/docs/configuration.md b/docs/configuration.md index 4de6ca7..1a3d8dc 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -62,6 +62,46 @@ to see what, if anything, is worth adding for your repo. |---|---|---|---| | `enabled` | `false` | Opt into the `cli_wiring` pre-commit step: every `[project.scripts]` entry must be reachable from a wiring source (install bootstrap, pre-commit, audit, hooks, agents, skills…). | Your repo ships `[project.scripts]` and follows forge's layout and you want unreachable CLIs caught. | +## `[tool.forge.precommit]` — turn steps on and off + +The uniform lever over the pre-commit sequence. Applied on top of each +step's own self-skip (it never *weakens* a self-skip), and `disable` beats +`enable` when a name appears in both. The same effect for a single run is +available as `forge-precommit --only ` / `--skip `. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `disable` | `[]` | Force-skip these default steps by name (e.g. `["pip_audit"]`). | You want a default step off repo-wide. | +| `enable` | `[]` | Opt into normally-off steps by name: `doctest`, `typecheck`, `doc_consistency`. | You want one of the opt-in steps below to run. | + +## `[tool.forge.doctest]` — opt-in doctest step + +Runs `pytest --doctest-modules` so docstring `>>>` examples are executed, +not just present. Enable via `[tool.forge.precommit] enable = ["doctest"]`. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `paths` | `["src"]` | Scan roots for `--doctest-modules`. | Your doctested code lives elsewhere. | +| `blocking` | `false` | Fail the commit on a broken example (else non-blocking WARN). | You want doctest drift to refuse a commit. | + +## `[tool.forge.typecheck]` — opt-in type-check step + +Runs [pyrefly](https://github.com/facebook/pyrefly) (Rust, stable, +pyproject-native, reads/migrates `[tool.mypy]`). Enable via +`[tool.forge.precommit] enable = ["typecheck"]`. When enabled but +`pyrefly` is absent, the step fails loudly (it does not silently pass). +Non-blocking by default — a type-checker false positive that refuses a +commit trains `--no-verify`. + +| Key | Default | What it does | Set it when | +|---|---|---|---| +| `paths` | `["src"]` | Scan roots passed to pyrefly. | Your source lives elsewhere. | +| `blocking` | `false` | Fail the commit on a type error (else non-blocking WARN). | Your type baseline is clean and you want it enforced. | + +The `doc_consistency` step (`verify-forge-doc-consistency`, enabled the +same way) has no config table — it checks that every `[project.scripts]` +CLI is documented in `docs/cli-reference.md`, and is always non-blocking. + ## `[tool.forge.docstring_coverage]` Forge-specific keys for the docstring-coverage reporter. (The coverage *gate* diff --git a/pyproject.toml b/pyproject.toml index bc4173a..c3c90a1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -31,6 +31,7 @@ verify-forge-repo-structure = "forge.verify_repo_structure:main" install-forge-labels = "forge.install_labels:main" verify-forge-test-naming = "forge.verify_test_naming:main" verify-forge-cli-wiring = "forge.verify_cli_wiring:main" +verify-forge-doc-consistency = "forge.verify_doc_consistency:main" forge-gen-cli-reference = "forge.gen_cli_reference:main" forge-gen-api-digest = "forge.gen_api_digest:main" forge-gen-commit-types = "forge.gen_commit_types:main" @@ -60,12 +61,14 @@ forge-audit-all = "forge.audit.all:main" test = ["pytest>=8.0"] audit = ["vulture>=2.10", "jsonschema>=4.0", "PyYAML>=6.0"] audit-tach = ["tach>=0.30"] +# Type checker for the opt-in `typecheck` pre-commit step (pyrefly only). +typecheck = ["pyrefly>=1,<2"] # Contributor / dev-loop umbrella. Aggregates the targeted extras so # every reference to `pip install -e ".[dev]"` in forge's own source, # docs, hook error messages, and test assertions resolves to a real # install target inside forge's own repo. Consumer repos typically # carry their own `[dev]` extra; this one is for forge itself. -dev = ["forge-scripts[test,audit]"] +dev = ["forge-scripts[test,audit,typecheck]"] [project.urls] Homepage = "https://github.com/misnaej/forge" diff --git a/src/forge/audit/claims.py b/src/forge/audit/claims.py index 8ff3dec..535e8c7 100644 --- a/src/forge/audit/claims.py +++ b/src/forge/audit/claims.py @@ -318,7 +318,7 @@ def _comment_findings( evidence=(comment_body[:COMMENT_PREVIEW],), ), ) - except tokenize.TokenizeError as exc: + except tokenize.TokenError as exc: logger.debug("tokenize failed in %s: %s", rel, exc) return findings diff --git a/src/forge/audit/dup.py b/src/forge/audit/dup.py index 67bc19e..20e8e04 100644 --- a/src/forge/audit/dup.py +++ b/src/forge/audit/dup.py @@ -177,7 +177,7 @@ def _tokenize_body(source: str) -> list[str]: tokens.append(tok.string if keyword.iskeyword(tok.string) else "ID") else: tokens.append(tok.string) - except tokenize.TokenizeError: + except tokenize.TokenError: logger.debug("tokenize failed on a snippet — skipping") return [] return tokens diff --git a/src/forge/audit/suppressions.py b/src/forge/audit/suppressions.py index 13205ff..263468b 100644 --- a/src/forge/audit/suppressions.py +++ b/src/forge/audit/suppressions.py @@ -283,7 +283,7 @@ def _iter_comments(text: str) -> list[tuple[int, str]]: seen_lines.add(line_no) if 1 <= line_no <= len(source_lines): pairs.append((line_no, source_lines[line_no - 1])) - except tokenize.TokenizeError as exc: + except tokenize.TokenError as exc: logger.debug("tokenize failed: %s", exc) return pairs diff --git a/src/forge/forge_config.py b/src/forge/forge_config.py index 75e72b7..e5a6a8c 100644 --- a/src/forge/forge_config.py +++ b/src/forge/forge_config.py @@ -101,6 +101,40 @@ class ConfigKey: "Per-tool override of the coverage scan roots; otherwise inherits " "the repo-wide [tool.forge].source_dirs + test_dirs.", ), + ConfigKey( + ("tool", "forge", "precommit", "disable"), + default=[], + description="Force-skip these pre-commit steps by name (over each " + "step's own self-skip).", + ), + ConfigKey( + ("tool", "forge", "precommit", "enable"), + default=[], + description="Opt into normally-off pre-commit steps by name " + "(doctest, typecheck, doc_consistency).", + ), + ConfigKey( + ("tool", "forge", "doctest", "paths"), + ["src"], + "Scan roots for the opt-in doctest step (pytest --doctest-modules).", + ), + ConfigKey( + ("tool", "forge", "doctest", "blocking"), + default=False, + description="Make the doctest step fail the commit on a broken " + "example (default: non-blocking WARN).", + ), + ConfigKey( + ("tool", "forge", "typecheck", "paths"), + ["src"], + "Scan roots for the opt-in pyrefly typecheck step.", + ), + ConfigKey( + ("tool", "forge", "typecheck", "blocking"), + default=False, + description="Make the typecheck step fail the commit on a checker " + "error (default: non-blocking WARN).", + ), ) # Third-party tools forge reads from their OWN native section rather than diff --git a/src/forge/precommit.py b/src/forge/precommit.py index eebdb1a..33a2cec 100644 --- a/src/forge/precommit.py +++ b/src/forge/precommit.py @@ -1,21 +1,32 @@ """forge-precommit — pre-commit dispatcher CLI. Generic Python pre-commit check dispatcher. Auto-detects source -directories and runs a fixed sequence on every commit: ruff (always -``ruff format`` + ``ruff check --fix --unsafe-fixes`` in-place, with -modified tracked files re-staged via ``git add``), docstring -verification (over the diff vs main), test-name verification (over the -diff vs main), repo-structure verification (``REPO_STRUCTURE.md`` vs -the actual tree), plugin manifest JSON validation, Claude Code -plugin-version drift guard (when applicable), and ``pip-audit`` -dependency vulnerability scan (non-blocking — warns but does not refuse -a commit). Shipped to consumers via the ``forge-scripts`` pip package -and invoked by ``.githooks/pre-commit`` after ``install-forge-githooks``. - -Pytest is intentionally NOT part of the default sequence — it is too -slow for pre-commit and belongs in CI. Consumers that want it on commit -can call ``pytest`` directly in ``.githooks/pre-commit`` around the -``forge-precommit`` invocation. +directories and runs an ordered sequence on every commit. The default +sequence: ruff (always ``ruff format`` + ``ruff check --fix +--unsafe-fixes`` in-place, with modified tracked files re-staged via +``git add``), docstring verification (over the diff vs main), test-name +verification (over the diff vs main), repo-structure verification +(``REPO_STRUCTURE.md`` vs the actual tree), plugin manifest JSON +validation, Claude Code plugin-version drift guard (when applicable), +and ``pip-audit`` dependency vulnerability scan (non-blocking — warns +but does not refuse a commit). Shipped to consumers via the +``forge-scripts`` pip package and invoked by ``.githooks/pre-commit`` +after ``install-forge-githooks``. + +Three further steps are **opt-in** (off by default): ``doctest`` +(``pytest --doctest-modules``), ``typecheck`` (``pyrefly``), and +``doc_consistency`` (doc claims vs repo state). Opt in by listing them in +``[tool.forge.precommit] enable``. The same table's +``disable`` list force-skips any default step; ``--only`` / ``--skip`` +do the same for a single run. This override layer sits on top of each +step's own self-skip — it never weakens one, and ``disable`` beats +``enable`` when a name appears in both. + +Pytest is intentionally NOT in the default sequence — it is too slow for +pre-commit and belongs in CI. The opt-in ``doctest`` step runs pytest +only over docstring examples in configured paths, and is non-blocking by +default for the same speed reason. Consumers that want the full suite on +commit call ``pytest`` directly in ``.githooks/pre-commit``. Step outputs are written to ``code_health/.log`` per FOUNDATION §13 so downstream tooling can read the latest results without re-running. @@ -24,6 +35,8 @@ - ``forge-precommit`` — run the default sequence - ``forge-precommit --json`` — machine-readable summary on stdout +- ``forge-precommit --only ruff,doctest`` — run just these steps +- ``forge-precommit --skip pip_audit`` — default sequence minus a step Consumers add repo-specific bash steps by editing ``.githooks/pre-commit`` directly — lines before the ``forge-precommit`` call run first; lines @@ -38,10 +51,10 @@ import shutil import subprocess import sys -import tomllib from dataclasses import asdict, dataclass from typing import TYPE_CHECKING +from forge import config from forge.git_utils import ( detect_existing_source_dirs, emit, @@ -52,8 +65,11 @@ if TYPE_CHECKING: + from collections.abc import Callable, Sequence from pathlib import Path + StepFn = Callable[[Path], "StepResult"] + # ANSI colors (suppressed if stdout isn't a TTY — keep machine-readable). def _color(code: str) -> str: @@ -100,6 +116,46 @@ class StepResult: non_blocking: bool = False +@dataclass(frozen=True) +class StepDef: + """A registry entry: a step's name, its function, and whether it runs by default. + + Co-locating the three properties keeps the step registry a single + source of truth — a name can't drift between a separate ordered list + and a separate default-on set. ``default_on=False`` marks an opt-in + step that runs only when listed in ``[tool.forge.precommit] enable`` + (or ``--only``). + + Attributes: + name: Step identifier used in config, CLI flags, and the log slug. + fn: The ``step_*`` callable producing this step's ``StepResult``. + default_on: Whether the step is part of the default sequence. + """ + + name: str + fn: StepFn + default_on: bool = True + + +def _forge_step_config(repo_root: Path, step: str) -> dict[str, object]: + """Return the ``[tool.forge.]`` table, or ``{}`` when absent. + + Single navigation point for the repeated ``tool → forge → `` + lookup every config-reading step performs. Uses the forgiving + :func:`config.read_pyproject_raw` loader, so a missing or malformed + ``pyproject.toml`` yields ``{}`` rather than raising. + + Args: + repo_root: Git repo root. + step: The ``[tool.forge.]`` subsection name (e.g. ``"doctest"``). + + Returns: + The subsection dict, or ``{}`` when any level is missing. + """ + data = config.read_pyproject_raw(repo_root) + return ((data.get("tool") or {}).get("forge") or {}).get(step) or {} + + def _run( cmd: list[str], cwd: Path, @@ -480,16 +536,7 @@ def _cli_wiring_enabled(repo_root: Path) -> bool: otherwise (including when ``pyproject.toml`` is missing or malformed). """ - pyproject = repo_root / "pyproject.toml" - if not pyproject.is_file(): - return False - try: - with pyproject.open("rb") as fh: - data = tomllib.load(fh) - except tomllib.TOMLDecodeError: - return False - section = ((data.get("tool") or {}).get("forge") or {}).get("cli_wiring") or {} - return bool(section.get("enabled")) + return bool(_forge_step_config(repo_root, "cli_wiring").get("enabled")) def step_plugin_version(repo_root: Path) -> StepResult: @@ -516,6 +563,150 @@ def step_plugin_version(repo_root: Path) -> StepResult: ) +def _bad_scan_paths(paths: list[object], repo_root: Path) -> list[str]: + """Return config scan-path values that are option-like or escape the repo. + + Config-supplied scan roots (``[tool.forge.] paths``) are spliced + into a subprocess argv. A value starting with ``-`` would be parsed as + a tool flag, and an absolute / ``..`` path would scan outside the repo. + Both are misconfiguration and are rejected rather than passed through. + + Args: + paths: The configured scan roots. + repo_root: Git repo root the paths must stay within. + + Returns: + The offending entries; empty when every path is a safe in-repo root. + """ + bad: list[str] = [] + for raw in paths: + if not isinstance(raw, str) or not raw.strip() or raw.startswith("-"): + bad.append(str(raw)) + continue + try: + (repo_root / raw).resolve().relative_to(repo_root.resolve()) + except ValueError: + bad.append(raw) + return bad + + +def step_doctest(repo_root: Path) -> StepResult: + """Run ``pytest --doctest-modules`` over docstring examples (opt-in). + + Executes the ``>>>`` examples embedded in docstrings so they cannot + rot silently — forge already enforces docstring presence (ruff D), + Args/Returns accuracy (``verify_docstrings``), and coverage + (``docstring_coverage``), but not example *execution*. Scans + ``[tool.forge.doctest] paths`` (default ``["src"]``). Non-blocking + unless ``blocking = true``: pytest is slow and a false failure that + refuses a commit trains ``--no-verify``. + + Opt-in only — runs when listed in ``[tool.forge.precommit] enable`` + (or ``--only``). When pytest output contains ``"no tests ran"`` (no + docstring examples collected), the step is treated as a skip rather + than a failure. + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult`` for the doctest run; ``non_blocking`` is the + inverse of ``[tool.forge.doctest] blocking``. + + Raises: + SystemExit: If ``pytest`` is not on PATH. + """ + cfg = _forge_step_config(repo_root, "doctest") + paths = cfg.get("paths") or ["src"] + blocking = bool(cfg.get("blocking", False)) + bad = _bad_scan_paths(paths, repo_root) + if bad: + return StepResult( + name="doctest", + passed=False, + output=f"invalid [tool.forge.doctest] paths: {bad}", + ) + require_cli("pytest", caller="forge-precommit") + passed, output = _run( + ["pytest", "--doctest-modules", "--doctest-continue-on-failure", *paths], + cwd=repo_root, + ) + # pytest>=8 prints "no tests ran" when zero examples were collected + # (exit 5) — a skip for an advisory doctest sweep, not a failure. + if "no tests ran" in output: + return StepResult(name="doctest", passed=True, output=output, skipped=True) + return StepResult( + name="doctest", passed=passed, output=output, non_blocking=not blocking + ) + + +def step_typecheck(repo_root: Path) -> StepResult: + """Run pyrefly over the source tree (opt-in). + + pyrefly is forge's type checker — same Astral model as ruff (single + Rust binary, pyproject-native, reads/migrates ``[tool.mypy]`` config), + so it slots into forge's existing toolchain with no Node runtime. + Scans ``[tool.forge.typecheck] paths`` (default ``["src"]``). + Non-blocking unless ``blocking = true``: a type-checker false positive + that refuses a commit trains ``--no-verify``, so the gate is advisory + by default. + + Opt-in only — runs when listed in ``[tool.forge.precommit] enable``. + When opted in but ``pyrefly`` is absent, fails loudly (the consumer + opted in and must install it) rather than silently passing. + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult`` mirroring pyrefly's exit code; ``non_blocking`` is + the inverse of ``blocking``. A blocking failure when the + configured ``paths`` are option-like or escape the repo. + + Raises: + SystemExit: If ``pyrefly`` is not on PATH. + """ + cfg = _forge_step_config(repo_root, "typecheck") + paths = cfg.get("paths") or ["src"] + blocking = bool(cfg.get("blocking", False)) + bad = _bad_scan_paths(paths, repo_root) + if bad: + return StepResult( + name="typecheck", + passed=False, + output=f"invalid [tool.forge.typecheck] paths: {bad}", + ) + require_cli("pyrefly", caller="forge-precommit") + passed, output = _run(["pyrefly", "check", *paths], cwd=repo_root) + return StepResult( + name="typecheck", passed=passed, output=output, non_blocking=not blocking + ) + + +def step_doc_consistency(repo_root: Path) -> StepResult: + """Run ``verify-forge-doc-consistency`` — doc claims vs repo state (opt-in). + + Checks the machine-checkable subset of documentation claims: every + ``[project.scripts]`` CLI name appears in ``docs/cli-reference.md``. + Opt-in via ``[tool.forge.precommit] enable``; non-blocking (doc drift + is a warning, not grounds to refuse a commit). + + Args: + repo_root: Git repo root. + + Returns: + ``StepResult`` mirroring the CLI exit code; always ``non_blocking``. + + Raises: + SystemExit: If ``verify-forge-doc-consistency`` is not on PATH. + """ + require_cli("verify-forge-doc-consistency", caller="forge-precommit") + passed, output = _run(["verify-forge-doc-consistency"], cwd=repo_root) + return StepResult( + name="doc_consistency", passed=passed, output=output, non_blocking=True + ) + + # --------------------------------------------------------------------------- # Dispatcher # --------------------------------------------------------------------------- @@ -555,44 +746,124 @@ def _print_step_line(result: StepResult) -> None: emit(f"{result.name:<24} {color}{marker}{NC}") +# Ordered registry — the single source of truth for which steps exist, +# their run order, and which are on by default. Opt-in steps +# (default_on=False) run only when named in [tool.forge.precommit].enable +# or --only. ruff is first because it mutates + re-stages files. +_STEP_REGISTRY: tuple[StepDef, ...] = ( + StepDef("ruff", step_ruff), + StepDef("docstring_verification", step_docstrings), + StepDef("docstring_coverage", step_docstring_coverage), + StepDef("test_naming_check", step_test_naming), + StepDef("repo_structure_check", step_repo_structure), + StepDef("manifest_json", step_manifest_json), + StepDef("cli_wiring", step_cli_wiring), + StepDef("commit_types_parity", step_commit_types_parity), + StepDef("plugin_version", step_plugin_version), + StepDef("pip_audit", step_pip_audit), + StepDef("doctest", step_doctest, default_on=False), + StepDef("typecheck", step_typecheck, default_on=False), + StepDef("doc_consistency", step_doc_consistency, default_on=False), +) + +_DEFAULT_ON: frozenset[str] = frozenset(d.name for d in _STEP_REGISTRY if d.default_on) + + +def _validate_step_names(names: Sequence[str]) -> None: + """Raise ``ValueError`` listing any *names* that are not registered steps. + + Args: + names: Step names referenced by config or CLI flags. + + Raises: + ValueError: When one or more names are unknown; the message names + the offenders and lists every valid step. + """ + unknown = sorted(set(names) - {d.name for d in _STEP_REGISTRY}) + if unknown: + valid = ", ".join(d.name for d in _STEP_REGISTRY) + msg = f"unknown step name(s): {unknown}. Valid names: {valid}" + raise ValueError(msg) + + +def _resolve_steps( + repo_root: Path, + *, + skip: Sequence[str] = (), + only: Sequence[str] = (), +) -> list[StepDef]: + """Resolve which steps to run, in registry order. + + ``--only`` selects the base set (those steps instead of the defaults); + otherwise the base set is the default-on steps plus + ``[tool.forge.precommit] enable``, minus ``[tool.forge.precommit] + disable``. ``skip`` then subtracts from **either** base — so + ``--only X --skip X`` runs nothing, and ``--skip`` is never silently + ignored. An explicit exclusion wins: ``disable`` / ``skip`` beat + ``enable`` when a name appears in both. Every referenced name is + validated against the registry; an unknown name raises ``ValueError`` + so the caller prints one clean message instead of leaking a traceback + on a config typo. + + Args: + repo_root: Git repo root. + skip: Step names to force-skip for this run (CLI ``--skip``). + only: When non-empty, the exact set of steps to run (CLI ``--only``). + + Returns: + The selected ``StepDef`` entries in registry order. + + Raises: + ValueError: When any referenced name is not a registered step. + """ + precommit_cfg = _forge_step_config(repo_root, "precommit") + enable = list(precommit_cfg.get("enable") or []) + disable = list(precommit_cfg.get("disable") or []) + _validate_step_names([*enable, *disable, *skip, *only]) + base = set(only) if only else (_DEFAULT_ON | set(enable)) - set(disable) + chosen = base - set(skip) + return [d for d in _STEP_REGISTRY if d.name in chosen] + + def run_all( repo_root: Path | None = None, *, print_progress: bool = True, + skip: Sequence[str] = (), + only: Sequence[str] = (), ) -> list[StepResult]: - """Run every step in order and return their results. + """Run the resolved step sequence in order and return their results. - ``step_ruff`` shells out to ``fix-forge-ruff`` which applies ruff - fixes (``ruff format`` + ``ruff check --fix --unsafe-fixes``) and - re-stages modified tracked files. Other steps verify only. + The sequence is resolved from the registry via :func:`_resolve_steps` + (``[tool.forge.precommit] enable/disable`` plus ``skip`` / ``only``). + ``step_ruff`` runs first and shells out to ``fix-forge-ruff`` (applies + ruff fixes and re-stages modified tracked files); the rest verify only. Args: repo_root: Override the auto-detected git repo root. Useful in tests. print_progress: Print one-line PASS/FAIL/SKIP per step. Disable for JSON output to keep stdout machine-readable. + skip: Step names to force-skip for this run. + only: When non-empty, run exactly these steps. Returns: - List of ``StepResult``, one per step, in execution order. + List of ``StepResult``, one per executed step, in execution order. + + Raises: + ValueError: When ``skip`` / ``only`` / config names an unknown step. """ root = repo_root if repo_root is not None else get_repo_root() results: list[StepResult] = [] - ruff_result = step_ruff(root) - if print_progress: - _print_step_line(ruff_result) - _write_log(root, ruff_result) - results.append(ruff_result) - for step in ( - step_docstrings, - step_docstring_coverage, - step_test_naming, - step_repo_structure, - step_manifest_json, - step_cli_wiring, - step_commit_types_parity, - step_plugin_version, - step_pip_audit, - ): - result = step(root) + this_module = sys.modules[__name__] + for step_def in _resolve_steps(root, skip=skip, only=only): + # Resolve each step by name through the module namespace rather than + # calling ``step_def.fn`` directly. The registry captured the + # original function objects at import time, so a test that does + # ``monkeypatch.setattr(precommit, "step_ruff", stub)`` would be + # invisible to a direct ``step_def.fn`` call. Re-resolving by name is + # the load-bearing seam that keeps per-step monkeypatching working. + fn = getattr(this_module, step_def.fn.__name__) + result = fn(root) if print_progress: _print_step_line(result) _write_log(root, result) @@ -600,6 +871,24 @@ def run_all( return results +def _split_csv(values: Sequence[str]) -> list[str]: + """Flatten repeatable / comma-separated CLI values into a clean name list. + + ``--skip a,b --skip c`` and ``--skip a --skip b --skip c`` both yield + ``["a", "b", "c"]``. Empty fragments (from stray commas) are dropped. + + Args: + values: Raw ``append``-collected argument values. + + Returns: + Flattened, stripped, non-empty tokens in order. + """ + out: list[str] = [] + for value in values: + out.extend(token.strip() for token in value.split(",") if token.strip()) + return out + + def main() -> int: """CLI entry point. @@ -626,9 +915,29 @@ def main() -> int: action="store_true", help="Emit a JSON summary on stdout instead of human output.", ) + parser.add_argument( + "--skip", + action="append", + default=[], + metavar="STEP[,STEP...]", + help="Force-skip these steps for this run (repeatable or comma-separated).", + ) + parser.add_argument( + "--only", + action="append", + default=[], + metavar="STEP[,STEP...]", + help="Run exactly these steps (repeatable or comma-separated).", + ) args = parser.parse_args() - results = run_all(print_progress=not args.json) + skip = _split_csv(args.skip) + only = _split_csv(args.only) + try: + results = run_all(print_progress=not args.json, skip=skip, only=only) + except ValueError as exc: + emit(f"{RED}forge-precommit: {exc}{NC}") + return 1 blocking_failures = [r for r in results if not r.passed and not r.non_blocking] non_blocking_warnings = [r for r in results if not r.passed and r.non_blocking] diff --git a/src/forge/verify_doc_consistency.py b/src/forge/verify_doc_consistency.py new file mode 100644 index 0000000..2a6f7c0 --- /dev/null +++ b/src/forge/verify_doc_consistency.py @@ -0,0 +1,106 @@ +"""verify-forge-doc-consistency — check machine-checkable doc claims vs repo state. + +Backs the opt-in ``doc_consistency`` pre-commit step. Nothing in forge +otherwise checks documentation for factual drift against the repo: a doc +can list a CLI that no longer exists (or omit a new one) and only a +careful human reread catches it. This CLI closes the **structured, +NLP-free subset** of that hole. + +Check (self-skips when its inputs are absent, so the CLI is safe in any +repo): + +- **CLI coverage** — every ``[project.scripts]`` entry name appears at + least once in ``docs/cli-reference.md``. A CLI added or removed without + a matching doc line is drift. + +Scope is deliberately conservative for v1: the one robust, no-NLP, +no-maintenance check. Name-list/count checks that depend on prose +phrasing, internal-link validation, and repo-state facts (visibility, +default branch) are intentionally out of scope — tracked separately. + +Exit code: ``0`` when consistent or nothing to check, ``1`` when any +drift is found. The ``doc_consistency`` step renders a non-zero result as +a non-blocking ``WARN``. +""" + +from __future__ import annotations + +import argparse +import logging +import sys +import tomllib +from typing import TYPE_CHECKING + +from forge.git_utils import configure_cli_logging +from forge.git_utils import repo_root as get_repo_root + + +if TYPE_CHECKING: + from pathlib import Path + + +configure_cli_logging() +logger = logging.getLogger(__name__) + + +def _check_cli_coverage(repo_root: Path) -> list[str]: + """Return findings for ``[project.scripts]`` names missing from the CLI reference. + + Skips silently when ``pyproject.toml`` has no ``[project.scripts]`` + table or ``docs/cli-reference.md`` is absent — a repo without either + has nothing to drift. + + Args: + repo_root: Git repo root. + + Returns: + One finding string per script name absent from the reference doc; + empty when consistent or not applicable. + """ + pyproject = repo_root / "pyproject.toml" + reference = repo_root / "docs" / "cli-reference.md" + if not pyproject.is_file() or not reference.is_file(): + return [] + try: + data = tomllib.loads(pyproject.read_text(encoding="utf-8")) + except (tomllib.TOMLDecodeError, OSError): + return [] + scripts = (data.get("project") or {}).get("scripts") or {} + if not scripts: + return [] + reference_text = reference.read_text(encoding="utf-8") + return [ + f"docs/cli-reference.md: no entry for [project.scripts] CLI '{name}'" + for name in sorted(scripts) + if name not in reference_text + ] + + +def main() -> int: + """CLI entry point. + + Returns: + ``0`` when the check is consistent (or nothing applies); ``1`` when + drift is found. + """ + argparse.ArgumentParser( + prog="verify-forge-doc-consistency", + description=( + "Check that every [project.scripts] CLI is documented in " + "docs/cli-reference.md. Non-blocking reporter for the " + "doc_consistency pre-commit step." + ), + ).parse_args() + + findings = _check_cli_coverage(get_repo_root()) + if findings: + logger.error("Documentation drift detected:") + for finding in findings: + logger.error(" - %s", finding) + return 1 + logger.info("Documentation claims consistent with repo state.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tests/audit/test_dup.py b/tests/audit/test_dup.py index 8ceab1e..b0e5418 100644 --- a/tests/audit/test_dup.py +++ b/tests/audit/test_dup.py @@ -107,6 +107,18 @@ def test_tokenize_body_collapses_strings_and_numbers() -> None: assert "42" not in tokens +def test_tokenize_body_returns_empty_on_token_error() -> None: + """Unterminated source is swallowed and yields ``[]``, not an exception. + + Regression: the handler caught ``tokenize.TokenizeError`` — a name that + does not exist in the stdlib (the real exception is + ``tokenize.TokenError``) — so a tokenization failure raised + ``AttributeError`` instead of being caught. The unclosed parenthesis + below makes ``tokenize`` raise ``TokenError`` at EOF. + """ + assert _tokenize_body("x = (1 +") == [] + + def test_shingles_returns_empty_when_shorter_than_k() -> None: """A token sequence below ``k`` length yields no shingles.""" assert _shingles(["a", "b"], k=5) == frozenset() diff --git a/tests/test_forge_config.py b/tests/test_forge_config.py index 5b20282..be9c888 100644 --- a/tests/test_forge_config.py +++ b/tests/test_forge_config.py @@ -2,17 +2,81 @@ from __future__ import annotations +import re +from pathlib import Path from typing import TYPE_CHECKING from forge import forge_config if TYPE_CHECKING: - from pathlib import Path - import pytest +_REPO_ROOT = Path(__file__).resolve().parents[1] + + +def _forge_source_files(*, exclude: str) -> dict[Path, str]: + """Return ``src/forge`` ``*.py`` sources keyed by path, minus *exclude*. + + Args: + exclude: Filename to drop (e.g. the declaration site itself). + + Returns: + Mapping of file path to its text for every other module. + """ + return { + path: path.read_text(encoding="utf-8") + for path in (_REPO_ROOT / "src" / "forge").rglob("*.py") + if path.name != exclude + } + + +def test_every_config_key_leaf_is_read_by_some_module() -> None: + """Every CONFIG_KEYS leaf appears in a reader module — no stale entries. + + #46: CONFIG_KEYS is forge's single declared enumeration of the + ``[tool.forge.*]`` keys forge reads. This couples the registry to + reality — a key whose leaf no longer appears as a string literal in + any ``src/forge`` module (other than ``forge_config.py``, the + declaration site) is a stale entry that makes ``forge-config --list`` + advertise a key nothing reads. + """ + sources = _forge_source_files(exclude="forge_config.py") + for key in forge_config.CONFIG_KEYS: + leaf = key.path[-1] + read_somewhere = any( + f'"{leaf}"' in text or f"'{leaf}'" in text for text in sources.values() + ) + assert read_somewhere, ( + f"CONFIG_KEYS leaf {leaf!r} ({'.'.join(key.path)}) is referenced " + f"by no src/forge module — stale registry entry?" + ) + + +def test_forge_step_config_sections_are_declared() -> None: + """Every ``[tool.forge.
]`` read via ``_forge_step_config`` is declared. + + #46 reverse direction: a new pre-commit step reading + ``[tool.forge.]`` without a CONFIG_KEYS entry would make + ``forge-config --list`` silently incomplete. Extract the section names + passed to ``precommit._forge_step_config`` and assert each is a + declared third-level token under ``[tool.forge]``. + """ + precommit_src = (_REPO_ROOT / "src" / "forge" / "precommit.py").read_text( + encoding="utf-8" + ) + sections = set( + re.findall(r'_forge_step_config\([^,]+,\s*"([^"]+)"\)', precommit_src) + ) + declared = {key.path[2] for key in forge_config.CONFIG_KEYS if len(key.path) >= 3} + undocumented = sections - declared + assert not undocumented, ( + f"[tool.forge.{sorted(undocumented)}] read via _forge_step_config but " + f"absent from CONFIG_KEYS" + ) + + def test_lookup_returns_value_and_unset() -> None: """`_lookup` returns nested values, or the `_UNSET` sentinel when absent.""" data = {"tool": {"forge": {"base_branch": "trunk"}}} diff --git a/tests/test_precommit.py b/tests/test_precommit.py index d5491c1..e058177 100644 --- a/tests/test_precommit.py +++ b/tests/test_precommit.py @@ -592,3 +592,353 @@ def _failing_test_naming(_root: object) -> precommit.StepResult: # Each failed step listed with its log path assert "docstring_verification: see code_health/docstring_verification.log" in out assert "test_naming_check: see code_health/test_naming_check.log" in out + + +# --------------------------------------------------------------------------- +# Step framework: registry, resolution, CLI overrides (#6) +# --------------------------------------------------------------------------- + + +def _write_pyproject(tmp_path: Path, body: str) -> None: + """Write *body* as ``pyproject.toml`` in *tmp_path* (config-test helper). + + Args: + tmp_path: Temporary directory path. + body: TOML content to write. + """ + (tmp_path / "pyproject.toml").write_text(body, encoding="utf-8") + + +def _names(step_defs: list[precommit.StepDef]) -> list[str]: + """Return the names of resolved ``StepDef`` entries (readability helper). + + Args: + step_defs: List of step definitions. + + Returns: + List of step names extracted from the definitions. + """ + return [d.name for d in step_defs] + + +def _present(monkeypatch: pytest.MonkeyPatch) -> None: + """Make every binary resolve on PATH (so ``require_cli`` passes).""" + monkeypatch.setattr(precommit.shutil, "which", lambda name: f"/usr/bin/{name}") + + +def test_forge_step_config_reads_section(tmp_path: Path) -> None: + """`_forge_step_config` returns the `[tool.forge.]` table.""" + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["lib"]\n') + assert precommit._forge_step_config(tmp_path, "doctest") == {"paths": ["lib"]} + + +def test_forge_step_config_missing_returns_empty(tmp_path: Path) -> None: + """`_forge_step_config` returns `{}` when the section or pyproject is absent.""" + assert precommit._forge_step_config(tmp_path, "doctest") == {} + _write_pyproject(tmp_path, "[tool.forge]\n") + assert precommit._forge_step_config(tmp_path, "doctest") == {} + + +def test_validate_step_names_accepts_known() -> None: + """`_validate_step_names` is silent for registered step names.""" + precommit._validate_step_names(["ruff", "doctest", "pip_audit"]) + + +def test_validate_step_names_rejects_unknown() -> None: + """`_validate_step_names` raises ValueError naming the offender and valid set.""" + with pytest.raises(ValueError, match="unknown step name") as exc: + precommit._validate_step_names(["ruff", "nope"]) + assert "nope" in str(exc.value) + assert "ruff" in str(exc.value) + + +def test_resolve_steps_default_excludes_opt_in(tmp_path: Path) -> None: + """The default run set is the default-on steps; opt-in steps stay out.""" + names = _names(precommit._resolve_steps(tmp_path)) + assert "ruff" in names + assert "doctest" not in names + assert "typecheck" not in names + assert "doc_consistency" not in names + + +def test_resolve_steps_enable_adds_opt_in(tmp_path: Path) -> None: + """`[tool.forge.precommit] enable` opts a normally-off step in.""" + _write_pyproject(tmp_path, '[tool.forge.precommit]\nenable = ["doctest"]\n') + assert "doctest" in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_disable_removes_default(tmp_path: Path) -> None: + """`[tool.forge.precommit] disable` force-skips a default step.""" + _write_pyproject(tmp_path, '[tool.forge.precommit]\ndisable = ["pip_audit"]\n') + assert "pip_audit" not in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_disable_beats_enable(tmp_path: Path) -> None: + """When a name is in both `enable` and `disable`, `disable` wins.""" + _write_pyproject( + tmp_path, + '[tool.forge.precommit]\nenable = ["doctest"]\ndisable = ["doctest"]\n', + ) + assert "doctest" not in _names(precommit._resolve_steps(tmp_path)) + + +def test_resolve_steps_skip_removes(tmp_path: Path) -> None: + """The `skip` argument removes a step for this run only.""" + assert "ruff" not in _names(precommit._resolve_steps(tmp_path, skip=["ruff"])) + + +def test_resolve_steps_only_overrides_in_registry_order(tmp_path: Path) -> None: + """`only=[...]` runs exactly those steps, ordered by the registry not the arg.""" + resolved = _names(precommit._resolve_steps(tmp_path, only=["pip_audit", "ruff"])) + assert resolved == ["ruff", "pip_audit"] + + +def test_resolve_steps_only_still_honors_skip(tmp_path: Path) -> None: + """`skip` subtracts from the `only` set too — it is never silently ignored.""" + resolved = _names( + precommit._resolve_steps( + tmp_path, only=["ruff", "pip_audit"], skip=["pip_audit"] + ) + ) + assert resolved == ["ruff"] + + +def test_resolve_steps_unknown_name_raises(tmp_path: Path) -> None: + """An unknown name in config / skip / only raises ValueError.""" + with pytest.raises(ValueError, match="unknown step name"): + precommit._resolve_steps(tmp_path, only=["bogus"]) + + +def test_split_csv_flattens_repeats_and_commas() -> None: + """`_split_csv` flattens repeated and comma-separated values, dropping blanks.""" + assert precommit._split_csv(["a,b", "c"]) == ["a", "b", "c"] + assert precommit._split_csv(["a, ,b"]) == ["a", "b"] + assert precommit._split_csv([]) == [] + + +def test_run_all_only_dispatches_monkeypatched_steps( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """run_all(only=...) runs exactly the named steps via the module dispatch. + + MOCK SETUP: ``step_ruff`` and ``step_pip_audit`` are replaced with + canned passing stubs; run_all is called with ``only`` those two. + EXPECTED BEHAVIOR: both stubs run (proving the registry resolves the + monkeypatched functions, not its captured references) and no other + step executes. + """ + + def _ruff(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="ruff", passed=True, output="x") + + def _audit(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="pip_audit", passed=True, output="x") + + monkeypatch.setattr(precommit, "step_ruff", _ruff) + monkeypatch.setattr(precommit, "step_pip_audit", _audit) + results = precommit.run_all( + tmp_path, print_progress=False, only=["ruff", "pip_audit"] + ) + assert [r.name for r in results] == ["ruff", "pip_audit"] + + +def test_main_only_flag_runs_subset( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """`--only ruff --json` runs just ruff and emits a one-entry JSON list. + + MOCK SETUP: get_repo_root pinned to tmp_path; ``step_ruff`` stubbed to + pass; argv drives main() with ``--only ruff --json``. + """ + monkeypatch.setattr(precommit, "get_repo_root", lambda: tmp_path) + + def _ruff(_root: object) -> precommit.StepResult: + return precommit.StepResult(name="ruff", passed=True, output="x") + + monkeypatch.setattr(precommit, "step_ruff", _ruff) + with patch.object( + precommit.sys, "argv", ["forge-precommit", "--only", "ruff", "--json"] + ): + rc = precommit.main() + assert rc == 0 + data = json.loads(capsys.readouterr().out) + assert [r["name"] for r in data] == ["ruff"] + + +def test_main_unknown_step_name_exits_one( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """An unknown `--skip` name prints a clean error and exits 1 (no traceback).""" + monkeypatch.setattr(precommit, "get_repo_root", lambda: tmp_path) + with patch.object(precommit.sys, "argv", ["forge-precommit", "--skip", "bogus"]): + rc = precommit.main() + assert rc == 1 + assert "unknown step name" in capsys.readouterr().out + + +# --------------------------------------------------------------------------- +# Opt-in steps: doctest (#5), typecheck (#48), doc_consistency (#4) +# --------------------------------------------------------------------------- + + +def test_step_doctest_passes_non_blocking_by_default( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest passes as non-blocking when `blocking` is unset. + + MOCK SETUP: pytest present on PATH; ``_run`` returns a passing canned + result; no ``[tool.forge.doctest]`` config so defaults apply. + """ + _present(monkeypatch) + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (True, "3 passed")) + result = precommit.step_doctest(tmp_path) + assert result.passed + assert result.non_blocking + assert not result.skipped + + +def test_step_doctest_uses_configured_paths( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest runs `pytest --doctest-modules` over `[tool.forge.doctest].paths`.""" + _present(monkeypatch) + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["lib", "app"]\n') + captured: dict[str, list[str]] = {} + + def _run(cmd: list[str], **_kw: object) -> tuple[bool, str]: + captured["cmd"] = cmd + return True, "1 passed" + + monkeypatch.setattr(precommit, "_run", _run) + precommit.step_doctest(tmp_path) + assert "--doctest-modules" in captured["cmd"] + assert captured["cmd"][-2:] == ["lib", "app"] + + +def test_step_doctest_no_examples_is_skipped( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Pytest 'no tests ran' (exit 5) counts as a skip, not a failure.""" + _present(monkeypatch) + monkeypatch.setattr( + precommit, "_run", lambda _cmd, **_kw: (False, "no tests ran in 0.01s") + ) + result = precommit.step_doctest(tmp_path) + assert result.skipped + assert result.passed + + +def test_step_doctest_blocking_config_is_blocking( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`blocking = true` makes a failing doctest a blocking failure.""" + _present(monkeypatch) + _write_pyproject(tmp_path, "[tool.forge.doctest]\nblocking = true\n") + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "1 failed")) + result = precommit.step_doctest(tmp_path) + assert not result.passed + assert not result.non_blocking + + +def test_step_doctest_missing_pytest_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Doctest fails loudly (SystemExit) when pytest is not on PATH.""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_doctest(tmp_path) + + +def test_step_typecheck_default_pyrefly( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Typecheck defaults to the `pyrefly check` command and is non-blocking. + + MOCK SETUP: pyrefly present on PATH; ``_run`` captures the command and + returns a passing result; no config so the default checker applies. + """ + _present(monkeypatch) + captured: dict[str, list[str]] = {} + + def _run(cmd: list[str], **_kw: object) -> tuple[bool, str]: + captured["cmd"] = cmd + return True, "0 errors" + + monkeypatch.setattr(precommit, "_run", _run) + result = precommit.step_typecheck(tmp_path) + assert captured["cmd"][:2] == ["pyrefly", "check"] + assert result.passed + assert result.non_blocking + + +def test_step_typecheck_missing_pyrefly_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """An opted-in-but-absent pyrefly binary fails loudly (SystemExit).""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_typecheck(tmp_path) + + +def test_step_typecheck_blocking_config( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """`blocking = true` makes a pyrefly error a blocking failure.""" + _present(monkeypatch) + _write_pyproject(tmp_path, "[tool.forge.typecheck]\nblocking = true\n") + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "error: x")) + result = precommit.step_typecheck(tmp_path) + assert not result.passed + assert not result.non_blocking + + +def test_step_typecheck_rejects_option_like_paths(tmp_path: Path) -> None: + """An option-like `paths` entry is rejected as a blocking misconfiguration.""" + _write_pyproject(tmp_path, '[tool.forge.typecheck]\npaths = ["--output=x"]\n') + result = precommit.step_typecheck(tmp_path) + assert not result.passed + assert not result.non_blocking + assert "--output=x" in result.output + + +def test_step_doctest_rejects_paths_escaping_repo(tmp_path: Path) -> None: + """A `paths` entry resolving outside the repo is rejected (blocking).""" + _write_pyproject(tmp_path, '[tool.forge.doctest]\npaths = ["/etc"]\n') + result = precommit.step_doctest(tmp_path) + assert not result.passed + assert not result.non_blocking + + +def test_step_doc_consistency_non_blocking( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """doc_consistency mirrors the CLI exit and is always non-blocking.""" + _present(monkeypatch) + monkeypatch.setattr(precommit, "_run", lambda _cmd, **_kw: (False, "drift")) + result = precommit.step_doc_consistency(tmp_path) + assert not result.passed + assert result.non_blocking + + +def test_step_doc_consistency_missing_cli_exits( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """doc_consistency fails loudly when its CLI is not on PATH.""" + monkeypatch.setattr(precommit.shutil, "which", lambda _name: None) + with pytest.raises(SystemExit): + precommit.step_doc_consistency(tmp_path) diff --git a/tests/test_verify_doc_consistency.py b/tests/test_verify_doc_consistency.py new file mode 100644 index 0000000..65ff5d5 --- /dev/null +++ b/tests/test_verify_doc_consistency.py @@ -0,0 +1,91 @@ +"""Tests for ``forge.verify_doc_consistency``. + +# MOCKING STRATEGY: each test builds a throwaway repo tree under tmp_path +# (pyproject, docs/cli-reference.md, agents/*.md, FOUNDATION.md) and runs +# the real check functions against it. ``main`` tests pin get_repo_root to +# tmp_path and patch sys.argv so argparse does not see pytest's argv. +""" + +from __future__ import annotations + +from typing import TYPE_CHECKING + +from forge import verify_doc_consistency as vdc + + +if TYPE_CHECKING: + from pathlib import Path + + import pytest + + +def _write(path: Path, text: str) -> None: + """Write *text* to *path*, creating parent directories as needed. + + Args: + path: Destination file path. + text: Contents to write. + """ + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text(text, encoding="utf-8") + + +def test_cli_coverage_skips_without_inputs(tmp_path: Path) -> None: + """No pyproject or no cli-reference → nothing to check.""" + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_cli_coverage_clean_when_all_documented(tmp_path: Path) -> None: + """Every `[project.scripts]` name present in the reference → no findings.""" + _write(tmp_path / "pyproject.toml", '[project.scripts]\nfoo-cli = "x:main"\n') + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n- foo-cli does things\n") + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_cli_coverage_flags_missing(tmp_path: Path) -> None: + """A script absent from the reference doc is reported by name.""" + _write( + tmp_path / "pyproject.toml", + '[project.scripts]\nfoo-cli = "x:main"\nbar-cli = "y:main"\n', + ) + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n- foo-cli only\n") + findings = vdc._check_cli_coverage(tmp_path) + assert len(findings) == 1 + assert "bar-cli" in findings[0] + + +def test_cli_coverage_malformed_pyproject_skips(tmp_path: Path) -> None: + """A malformed pyproject yields no findings rather than raising.""" + _write(tmp_path / "pyproject.toml", "this is = = not [[[ toml") + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n") + assert vdc._check_cli_coverage(tmp_path) == [] + + +def test_main_returns_zero_when_consistent( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """main() exits 0 on an empty repo — the check skips, nothing drifts. + + MOCK SETUP: get_repo_root pinned to an empty tmp_path; argv patched so + argparse does not consume pytest's arguments. + """ + monkeypatch.setattr(vdc, "get_repo_root", lambda: tmp_path) + monkeypatch.setattr(vdc.sys, "argv", ["verify-forge-doc-consistency"]) + assert vdc.main() == 0 + + +def test_main_returns_one_on_drift( + tmp_path: Path, + monkeypatch: pytest.MonkeyPatch, +) -> None: + """main() exits 1 when CLI coverage drifts. + + MOCK SETUP: a repo with a [project.scripts] CLI absent from the + reference doc; get_repo_root pinned to it and argv patched. + """ + _write(tmp_path / "pyproject.toml", '[project.scripts]\nundocumented = "x:main"\n') + _write(tmp_path / "docs" / "cli-reference.md", "# CLIs\n") + monkeypatch.setattr(vdc, "get_repo_root", lambda: tmp_path) + monkeypatch.setattr(vdc.sys, "argv", ["verify-forge-doc-consistency"]) + assert vdc.main() == 1