Skip to content

The book's contributing guide tells you to run the pre-commit framework the developer guide forbids #464

Description

@vladimirrott

Rechecked at ba1a7b8f on 2026-09-21. The defect is unchanged and the
required gate is still green over it. Two anchors were off by one, and the
trap paragraph overstated what docs/developer-guide.md contains; both
corrected below.

docs/contributing/CONTRIBUTING.md:128-129 tells a contributor to run the
pre-commit framework as the last step before pushing:

$ sed -n '117,130p' docs/contributing/CONTRIBUTING.md
### 3. Check Locally

```sh
# Required before every push
cargo fmt --all --check
cargo clippy --workspace --all-features --locked -- -D warnings
cargo nextest run --workspace --locked

# Frontend
npm test --prefix apps/sysknife-shell && npm exec --prefix apps/sysknife-shell -- tsc --noEmit

# All pre-commit hooks
pre-commit run --all-files
```

docs/developer-guide.md:69-72 forbids that setup by name:

$ sed -n '68,71p' docs/developer-guide.md
> Do **not** use `pip install pre-commit && pre-commit install`. This repository
> drives its hooks through `core.hooksPath`, so anything written into
> `.git/hooks` is ignored by Git and you would end up with no gate at all.
> `.pre-commit-config.yaml` is left over from an earlier setup and is not used.

Both pages ship in the book, so a reader can land on either:

$ grep -n 'contributing/CONTRIBUTING.md\|developer-guide' docs/SUMMARY.md
45:- [Developer Guide](developer-guide.md)
47:- [Contributing](contributing/CONTRIBUTING.md)

Why it matters

The comment says "All pre-commit hooks". The framework runs a different set of
commands from the hook that guards a commit, and they share one line:

$ python3 - <<'PY'
import re, pathlib
root = pathlib.Path('.')
hook = [l.strip() for l in (root/'.githooks/pre-commit').read_text().splitlines()
        if l.strip() and not l.lstrip().startswith(('#','set ','cd ','echo '))]
print("githooks/pre-commit runs:")
for c in hook: print("   ", c)
cfg = (root/'.pre-commit-config.yaml').read_text()
ids = re.findall(r'^\s*- id: (\S+)', cfg, re.M)
print("\n.pre-commit-config.yaml declares %d hook ids:" % len(ids))
print("   ", ", ".join(ids))
for e in re.findall(r'^\s*entry: (.+)$', cfg, re.M):
    print("    entry:", e)
PY
githooks/pre-commit runs:
    scripts/check_no_secrets.sh --staged
    cargo fmt --all --check
    cargo clippy --workspace --all-features --all-targets --locked -- -D warnings
    RUSTDOCFLAGS="-D warnings" cargo doc --no-deps --workspace --locked
    scripts/test_baseline.sh

.pre-commit-config.yaml declares 16 hook ids:
    trailing-whitespace, end-of-file-fixer, check-yaml, check-toml, check-json, check-merge-conflict, check-added-large-files, detect-private-key, mixed-line-ending, no-commit-to-branch, gitleaks, cargo-fmt, cargo-check, tsc, markdownlint, yamllint
    entry: cargo fmt --all -- --check
    entry: cargo check --workspace --locked
    entry: bash -c 'cd apps/sysknife-shell && npx tsc --noEmit'
    entry: bash -c 'npx --yes markdownlint-cli2 "$@"' --
    entry: yamllint

The framework runs cargo check where the gate runs
cargo clippy -- -D warnings, and it runs none of cargo doc under
RUSTDOCFLAGS="-D warnings", scripts/test_baseline.sh, or
scripts/check_no_secrets.sh --staged. A green pre-commit run --all-files
leaves the staged-secrets screen, the rustdoc screen and the test-count baseline
unrun, and tells you the opposite. .githooks/pre-commit says why that costs
something, in its own header:

$ sed -n '4,8p' .githooks/pre-commit
# These are the Rust checks the `rust` job in .github/workflows/ci.yml runs,
# copied verbatim so a green commit here means a green job there. The hook used
# to run only fmt + nextest, which let a broken intra-doc link (a doc comment
# pointing at a function the same commit deleted) pass locally and fail in CI.
# A local gate that is a subset of the remote one buys a false green, not speed.

#430 added a checker for this contradiction, and it reads one file:

$ sed -n '810,812p;840p' scripts/check_evidence_claims.py
def check_pre_commit_commands(root: Path, guide: str) -> list[str]:
    """Compare the documented gate with the executable hook, in order."""
    hook = root / ".githooks/pre-commit"
        problems += check_pre_commit_commands(root, texts["docs/developer-guide.md"])

So the required gate is green at fab9053f with the forbidden instruction
sitting in the book:

$ git rev-parse --short=8 HEAD
fab9053f
$ out="$(bash tests/release/public-claims.test.sh 2>&1)"; rc=$?; echo "rc=$rc"; printf '%s\n' "$out" | tail -3
rc=0
Published figures match the evidence artifacts.
Public claims are internally consistent.
Public claims contract passed.

Scope

  • Drop the # All pre-commit hooks block from
    docs/contributing/CONTRIBUTING.md. What belongs there instead, if anything,
    is bash .githooks/pre-commit, which the developer guide already names as the
    way to run the gate by hand:

    $ sed -n '273p' docs/developer-guide.md
    Run `bash .githooks/pre-commit` to invoke the same gate manually.
    
  • Extend scripts/check_evidence_claims.py so it screens every documentation
    file for the forbidden command instead of comparing one file's command list.
    Today a second copy of the instruction can appear anywhere and nothing objects.

  • The trap. docs/developer-guide.md:69 and .pre-commit-config.yaml:3
    both contain pre-commit install on purpose, and .pre-commit-config.yaml:4
    carries pre-commit run --all-files: one is the prohibition itself, the other
    comments a file the repo keeps and does not use. docs/developer-guide.md
    does not contain pre-commit run --all-files at all. A bare grep for those
    strings goes red on the two places that are already correct. Screen the command
    where it appears as a command, inside a shell fence, and leave prose and YAML
    comments alone.

  • Root CONTRIBUTING.md and docs/contributing/CONTRIBUTING.md are two separate
    documents that have drifted apart. Whether to merge them is a larger call and
    is not part of this.

Tests first

tests/release/public-claims.test.sh already builds a fixture tree and asserts
the checker rejects a mutation of it, including for this same hook-versus-prose
rule:

$ sed -n '98,102p' tests/release/public-claims.test.sh
# Hook/prose drift must name the changed command, not merely fail on a fixture.
printf '\nnode --version\n' >> "$fixture/.githooks/pre-commit"
assert_rejected_with_diagnostic 'undocumented sixth hook step' 'pre-commit steps differ' 'node --version'
cp "$repo_root/.githooks/pre-commit" "$fixture/.githooks/pre-commit"
sed '/^scripts\/test_baseline.sh$/d' "$repo_root/.githooks/pre-commit" > "$fixture/.githooks/pre-commit"

Add docs/contributing/CONTRIBUTING.md to fixture_files, plant
pre-commit run --all-files in a shell fence in the fixture copy, and assert the
checker exits 1. assert_rejected_with_diagnostic takes the strings the message
must contain, so pin the path as well as the rule; a diagnostic that says
"forbidden command" without saying where leaves the reader to search six
files.

Three things the PR has to show:

  1. The guard bites. Delete your new call site from main() in
    scripts/check_evidence_claims.py, re-run
    bash tests/release/public-claims.test.sh, and it goes red.
  2. The guard does not over-fire. With the prohibition paragraph in
    docs/developer-guide.md and the header of .pre-commit-config.yaml
    untouched, the checker exits 0 on a clean tree. Write that as its own
    assertion. The pristine-fixture check at line 76 only covers the paths listed
    in fixture_files.
  3. Order matters. The pristine fixture has to pass, so fix the document
    before adding its path to fixture_files. Do it the other way round and the
    test fails on the defect it exists to catch, which reads identically to a
    broken fixture.

Difficulty

easy. One block removed from a Markdown file, one function in a Python script
you can run on its own with python3 scripts/check_evidence_claims.py ., and the
fixture machinery to hang the assertion on already exists.

Getting started

CONTRIBUTING.md
has the build and test commands. No CLA and no copyright waiver. The project is MIT.

Related and separate: #451 covers which gate governs a contributor who cannot
build the GUI crate, and PR #457 is open against it. Neither touches this file.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    documentationImprovements or additions to documentationeasyDifficulty: self-contained, no deep context neededgood first issueGood for newcomershelp wantedExtra attention is needed

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions