Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6,816 changes: 6,816 additions & 0 deletions .craftsmanship-baseline.json

Large diffs are not rendered by default.

35 changes: 35 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -558,6 +558,40 @@ jobs:
- name: Check workflow files (actionlint + shellcheck)
run: actionlint -color

# Deterministic enforcement of CLAUDE.md § Code Style — file size, method
# size, layer-boundary imports, and unsourced magic numbers — which that
# section admitted was "enforced by code review today; no automated
# pre-commit hook checks this yet" until this job. Measured on a single
# PR the night before this job was added: a 301-line file reported as
# 280, three of four over-40-line methods unseen, one layer violation
# justified by a fabricated citation — each caught only by a human or
# agent re-reading the diff, never by a machine. See
# scripts/check_craftsmanship.py's module docstring for the rules and
# scripts/craftsmanship_baseline.py for why pre-existing debt (recorded
# in .craftsmanship-baseline.json) does not retroactively block.
craftsmanship:
name: Craftsmanship Gate
runs-on: ubuntu-latest
steps:
# fetch-depth: 0 so `origin/main` — the diff base the gate compares
# the PR's changed files against — is resolvable locally, not just
# the single commit a shallow checkout would leave.
- uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1
with:
fetch-depth: 0

- name: Set up Python
uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0
with:
python-version: "3.12"

# Standard library only (scripts/check_craftsmanship.py's module
# docstring) — no dependency install needed, matching the
# doc-claim/version-surface/ci-gate-completeness gates in `lint`
# below, which are static for the same reason.
- name: Run the craftsmanship gate on this PR's changed files
run: python scripts/check_craftsmanship.py

typecheck:
name: Type Check
runs-on: ubuntu-latest
Expand Down Expand Up @@ -767,6 +801,7 @@ jobs:
- mcp-host-config
- test-windows
- release-deps
- craftsmanship
- lint
- typecheck
- build
Expand Down
63 changes: 45 additions & 18 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,26 +77,53 @@ separate **cortex-viz** MCP (reads this same store read-only).

- 300 lines max per file; 40 lines max per method — a local tightening of
coding-standards.md §4.1/§4.2 (≤500/≤50; CONTRIBUTING.md § Code Style
cites the same 300/40 numbers). Enforced by code review today; no
automated pre-commit hook checks this yet (issue #276 corrected the
prior claim of a "craftsmanship-checker" hook — none exists in
`.git/hooks/` or a `.pre-commit-config.yaml`).
- Import rule: `core/` imports only `shared/` + stdlib; `infrastructure/`
never imports core/handlers. Verify both directions before every PR —
`grep -rn "from mcp_server.infrastructure" mcp_server/core/` and
`grep -rn "from mcp_server\.core\." mcp_server/infrastructure/*.py` —
both currently return nothing (re-verified 2026-08-10 while fixing
issue: `wiki_store.py`/`wiki_schema_reader.py` importing `core/`,
PR #409 round 3). The three violations this line used to name
(`wiki_axis_registry.py`, `wiki_classifier.py`, `wiki_schema_loader.py`,
found 2026-07-14 during #114) no longer exist: `wiki_schema_loader.py`
moved `core/` → `shared/` in the same fix, and the other two do not
import `infrastructure/` as of this measurement. Treat a future zero
as the standard, not as evidence the check is unnecessary — re-run the
greps, don't assume they still pass.
cites the same 300/40 numbers).
- Import rule: a TRUE whitelist per layer, all eight rows of
`docs/module-inventory.md` § Dependency Rules — `shared/` and `core/`
are pure (no third-party imports at all; `core/` additionally bans
`os`/`pathlib` even though they are stdlib, since it is zero-I/O
business logic); `infrastructure/`, `validation/`, `handlers/`,
`server/`, `hooks/` are boundary/adapter layers where third-party
imports are the point, but their `mcp_server.<layer>` cross-references
are still checked against the table's named whitelist, not a blacklist
of a few forbidden ones. This replaces the former manual-grep
verification step (`grep -rn "from mcp_server.infrastructure"
mcp_server/core/`, etc.) — the craftsmanship gate below runs it, in
both directions, across all eight layers, on every push and PR, so
"re-run the greps before every PR" is no longer the standard: the gate
is.
- No invented constants: every hardcoded number carries a `# source:`
comment (paper, committed benchmark, or dated measurement naming the
environment and conditions). A number without one blocks the diff in review.
environment and conditions).
- **Enforced by `scripts/check_craftsmanship.py`**, run in CI on every push
and PR (`.github/workflows/ci.yml`, `craftsmanship` job) and locally via
`python scripts/check_craftsmanship.py`. It checks the four rules above,
by AST, on the files a diff touches — never the whole repository. The
layer whitelist is *parsed* from `docs/module-inventory.md`'s own table
at run time (`scripts/craftsmanship_layer_table.py`), never a second
hardcoded copy that could silently diverge from it.
The comparison baseline is read via `git show <base-ref>:.craftsmanship-baseline.json`
— the PR's BASE ref, immutable to the PR's own commits — never the
working tree: a working-tree-only baseline is self-service (add a
violation, run `--write-baseline` in the same tree, the gate would pass
on it — this exact exploit is reproduced and closed in
`tests_py/scripts/test_check_craftsmanship.py::SneakyLimitExploitTests`).
The gate fails the diff on: any violation absent from that base-ref
baseline (new debt); any base-ref-baselined entry whose violation no
longer reproduces (fixed but not pruned); or any entry present in the
working-tree `.craftsmanship-baseline.json` but absent from the base
ref's (the file may only SHRINK within a PR — an addition is refused
outright, matched or not, because debt discovered mid-PR gets fixed at
the source, not grandfathered). Regenerate with
`python scripts/check_craftsmanship.py --write-baseline` only to prune
entries whose violations you actually fixed. Previously "enforced by
code review today; no automated pre-commit hook checks this yet" (issue
#276 corrected an earlier, false claim of a "craftsmanship-checker"
hook) — that gap is what this gate closes. Historical import-rule
violations once tracked ad hoc in this section (`wiki_axis_registry.py`,
`wiki_classifier.py`, `wiki_schema_loader.py`, found 2026-07-14 during
#114) now live in the baseline like any other pre-existing debt, not as
separate prose here.

## What NOT to do

Expand Down
9 changes: 9 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,15 @@ As of issue #178, every Cortex release ships with verifiable provenance
`Docker Smoke`, `Test` on Python 3.10–3.13, `Test (SQLite backend)`, and
`Test (Windows, SQLite backend)`). Force-pushes and branch deletion are
blocked, and conversation resolution is required.
`ci.yml`'s own aggregate gate (`CI Green`, the single context branch
protection actually names — see `scripts/check_ci_gate_complete.py`)
additionally runs a `Craftsmanship Gate` job as of the PR that added
`scripts/check_craftsmanship.py`; this number becomes twelve once a
repository admin adds it to the required-checks list in GitHub's branch
protection settings (Settings → Branches) — an action this document
cannot perform and the PR that introduced the job does not perform
either, so the two may legitimately disagree until that setting is
updated by hand.

### Why Scorecard's Code-Review check will not go green here

Expand Down
207 changes: 207 additions & 0 deletions scripts/check_craftsmanship.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,207 @@
"""Craftsmanship gate: a deterministic pass/fail check for the rules
``CLAUDE.md`` § Code Style states but — until this script — nothing
verified. See ``craftsmanship_rules.py`` for what each rule checks and why
its violation identifier is stable; see ``craftsmanship_baseline.py`` for
the ratchet that lets pre-existing debt through without blocking new debt,
and for why the comparison source is the PR's BASE ref, never the working
tree — a working-tree baseline can be tampered with in either direction
(add a violation and self-regenerate; or hand-delete an entry and leave
the violation in place), both reproduced and closed, see that module's
docstring. Git plumbing (resolving the base ref, reading the baseline as
committed there) lives in ``craftsmanship_git.py``.

Scope: by default, only the files a PR's diff touches (never the whole
repository) — a file untouched by this change is not this change's
problem. ``--write-baseline`` is the one mode that scans everything, because
regenerating the baseline is exactly the operation that must see the whole
tree.

Usage::

python scripts/check_craftsmanship.py # diff vs origin/main
python scripts/check_craftsmanship.py --base main # diff vs an explicit ref
python scripts/check_craftsmanship.py path/to/file.py # explicit files
python scripts/check_craftsmanship.py --write-baseline # regenerate the baseline

Exit codes: 0 clean, 1 new/stale/added/falsified-removal violations found,
2 could not determine which files to check (git diff failed and no files
were given explicitly) or could not resolve the base ref in diff mode.
"""

from __future__ import annotations

import argparse
import sys
from pathlib import Path

_SCRIPTS_DIR = str(Path(__file__).resolve().parent)
if _SCRIPTS_DIR not in sys.path:
sys.path.insert(0, _SCRIPTS_DIR)
import craftsmanship_rules as rules # noqa: E402
import craftsmanship_baseline as baseline_mod # noqa: E402
import craftsmanship_git # noqa: E402

REPO_ROOT = Path(__file__).resolve().parent.parent
DEFAULT_BASELINE = REPO_ROOT / ".craftsmanship-baseline.json"


def scan_files(rel_paths: list[str]) -> set[rules.Violation]:
"""Scan each path (relative to REPO_ROOT); a missing file yields nothing."""
found: set[rules.Violation] = set()
for rel_path in rel_paths:
full_path = REPO_ROOT / rel_path
if not full_path.is_file():
continue
source = full_path.read_text(encoding="utf-8")
found.update(rules.scan_source(rel_path, source))
return found


def _report(
new: list[rules.Violation],
stale: list[rules.Violation],
added: list[rules.Violation],
falsified: list[rules.Violation],
) -> None:
if added:
print(
"Craftsmanship gate: baseline entries ADDED without a base-ref "
"match (the ratchet only shrinks — fix the violation, don't "
"grandfather it):",
file=sys.stderr,
)
for v in added:
print(f" - [{v.kind}] {v.file}: {v.detail}", file=sys.stderr)
if falsified:
print(
"Craftsmanship gate: baseline entries REMOVED but the violation "
"still reproduces (a falsified prune — fix the code, or leave "
"the entry, don't just delete the JSON line):",
file=sys.stderr,
)
for v in falsified:
print(f" - [{v.kind}] {v.file}: {v.detail}", file=sys.stderr)
if new:
print(
"Craftsmanship gate: NEW violations (not in the base-ref baseline):",
file=sys.stderr,
)
for v in new:
print(f" - [{v.kind}] {v.file}: {v.detail}", file=sys.stderr)
if stale:
print(
"Craftsmanship gate: STALE baseline entries "
"(fixed in code but still listed — prune them):",
file=sys.stderr,
)
for v in stale:
print(f" - [{v.kind}] {v.file}: {v.detail}", file=sys.stderr)
if not new and not stale and not added and not falsified:
print("Craftsmanship gate: OK")


def _write_baseline(baseline_path: Path) -> int:
files = craftsmanship_git.all_tracked_python_files(REPO_ROOT)
violations = scan_files(files)
baseline_mod.save_baseline(baseline_path, violations)
print(f"Wrote {len(violations)} violation(s) to {baseline_path}")
for kind, count in baseline_mod.count_by_kind(violations).items():
print(f" {kind}: {count}")
return 0


def _added_and_falsified(
working_baseline: set[rules.Violation], base_baseline: set[rules.Violation] | None
) -> tuple[list[rules.Violation], list[rules.Violation]]:
"""The two base-ref-anchored ratchet checks — skipped (empty) in the
bootstrap case (no base-ref baseline to compare against yet).
"""
if base_baseline is None:
return [], []
added = baseline_mod.added_entries(working_baseline, base_baseline)
removed_files = sorted({v.file for v in base_baseline - working_baseline})
removed_rescanned = {f: scan_files([f]) for f in removed_files}
falsified = baseline_mod.falsified_removals(
base_baseline, working_baseline, removed_rescanned
)
return added, falsified


def _run_gate(
target_files: list[str], baseline_path: Path, base_ref: str | None
) -> int:
current = scan_files(target_files)
working_baseline = baseline_mod.load_baseline(baseline_path)
base_baseline = craftsmanship_git.load_baseline_from_ref(
REPO_ROOT, base_ref, baseline_path
)

# The tamper-proof comparison source for "is this violation already
# known" is the base ref's baseline — falling back to the working
# tree's only in the bootstrap case (base_baseline is None: no base
# ref, or the file does not exist at the base ref yet).
comparison_baseline = (
base_baseline if base_baseline is not None else working_baseline
)
new = baseline_mod.new_violations(current, comparison_baseline)
added, falsified = _added_and_falsified(working_baseline, base_baseline)

baseline_files = sorted({v.file for v in working_baseline})
rescanned = {f: scan_files([f]) for f in baseline_files}
stale = baseline_mod.stale_entries(working_baseline, rescanned)

_report(new, stale, added, falsified)
return 1 if (new or stale or added or falsified) else 0


def _parse_args(argv: list[str] | None) -> argparse.Namespace:
parser = argparse.ArgumentParser(description=__doc__)
parser.add_argument("files", nargs="*", help="explicit files to check")
parser.add_argument("--base", default=None, help="git ref to diff/compare against")
parser.add_argument(
"--baseline", default=str(DEFAULT_BASELINE), help="baseline JSON path"
)
parser.add_argument(
"--write-baseline",
action="store_true",
help="regenerate the baseline from the full tree",
)
return parser.parse_args(argv)


def main(argv: list[str] | None = None) -> int:
args = _parse_args(argv)
baseline_path = Path(args.baseline)

if args.write_baseline:
return _write_baseline(baseline_path)

if args.files:
# Ad hoc/local usage: the base ref still feeds the ratchet-file
# and base-baseline comparisons below when it resolves, but an
# unresolved ref here is NOT fatal (offline/no-remote local runs
# stay usable) — it just falls back to bootstrap semantics.
target_files = args.files
base_ref = craftsmanship_git.resolve_base_ref(REPO_ROOT, args.base)
else:
base_ref = craftsmanship_git.resolve_base_ref(REPO_ROOT, args.base)
if base_ref is None:
print(
"Craftsmanship gate: could not resolve a base ref to diff against",
file=sys.stderr,
)
return 2
diffed = craftsmanship_git.changed_python_files(REPO_ROOT, base_ref)
if diffed is None:
print(
f"Craftsmanship gate: `git diff` against {base_ref} failed",
file=sys.stderr,
)
return 2
target_files = diffed

return _run_gate(target_files, baseline_path, base_ref)


if __name__ == "__main__":
raise SystemExit(main())
Loading
Loading