Add tox for Python test/lint matrix + Rust MSRV verification - #31
Merged
Conversation
Nothing verified tools/*.py across Python versions before this — no pytest suite, no CI job, just a handful of ad-hoc --help/smoke commands in Makefile's test-python target. Adds: - tests/conftest.py: run_tool/home/git_repo fixtures — every test runs the real CLI as a subprocess against an isolated $HOME (and, for the two tools that shell out to git, an isolated git repo), so nothing touches the machine's real ~/.claude/ - tests/test_*.py: one file per tool, 44 tests total, functional not just --help (add/list/search/show roundtrips, error paths, exit codes) - tox.ini: py38-py313 (skip_missing_interpreters), lint (ruff check), fmt-check (ruff format --check) - pyproject.toml: scoped ruff config — E/F/I/UP only, not the full default ruleset (which includes bandit/blind-except/naive-datetime rules that fight this codebase's deliberate style), E402 ignored since every tool puts VERSION right after the docstring before imports - fmt-check / scripts/fmt.sh scoped to tools/claude-lessons.py + tests/ only, not all of tools/: the older tools use a deliberate hand-aligned style (aligned `=`, aligned dict values) that `ruff format` would flatten repo-wide — not forcing that as a side effect of adding tox - make tox: runs the full matrix (requires: pip install tox ruff) Verified locally: tox (py39/py312/py313 available here, py38/py310/py311 skip cleanly), lint, and fmt-check all pass.
Real, independently-reproduced bugs the new test suite caught: - claude-cost.py: `set-budget` crashed with FileNotFoundError on a fresh install — `_save_budget` never created ~/.claude/ before writing cost-budget.json (snippet.py and claude-handoff.py already did this correctly; claude-cost.py's budget path was the one that didn't). - claude-remind.py / claude-review-diff.py: both use `X | None` (PEP 604) type annotations, which raise TypeError at import time on Python < 3.10 — every subcommand, including --help, was broken on 3.8/3.9 despite the "Python 3.8+" badge. Fixed with `from __future__ import annotations` (defers annotation evaluation, no behavior change). Confirmed via tox -e py39 before/after. Also applies the (small, low-risk) findings from the new ruff lint gate across tools/*.py: 4 unused imports, 1 dead local variable (claude-handoff.py's `list`, left over from a removed date column), 4 `if x: y` one-liners split to satisfy E701, and one ambiguous single- letter loop variable (`l` -> `line`). No behavior changes; tests still pass after each.
- python-tests job: runs `tox -e py` across Python 3.8-3.13 via actions/setup-python (this is what actually caught the claude-remind.py/claude-review-diff.py 3.8/3.9 breakage in the previous commit — nothing exercised those versions before) - python-lint job: `tox -e lint,fmt-check` - rust-version = "1.75" declared in claude-tools/Cargo.toml, matching the README's existing "Rust 1.75+" badge (previously just a claim, not enforced anywhere) - msrv job: `cargo msrv verify` — actually builds against the declared 1.75 toolchain instead of only ever testing against `stable` No cargo/rustc available in the environment this branch was authored in, so the Rust-side change (rust-version field + msrv CI job) is verified by CI on push, the same as the rest of this project's Rust checks.
README.md/.ko.md: Makefile section (make tox, make msrv), repo layout (tests/, tox.ini, pyproject.toml). Also fixes "make lint # clippy + ruff" in both READMEs — the lint target only ever ran clippy; ruff wasn't wired into it before this branch. docs/CONTRIBUTING.md/.ko.md: tox usage in Development Setup, tests/ step added to "Adding a New Tool", tox/cargo-msrv checks added to the PR checklist, cargo-msrv install/verify commands. Closes #30
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
CI caught this immediately: cargo-msrv installs rustc 1.75 per the declared rust-version, but that toolchain's bundled cargo can't parse Cargo.lock's `version = 4` format (stabilized in cargo 1.78) — a structural incompatibility independent of whether the crate's actual code needs 1.75 or newer. The pre-existing "Rust 1.75+" badge this number was copied from was apparently never verified against the checked-in lockfile either. Bumped rust-version to 1.78 (README/README.ko badges updated to match) and letting CI's msrv job re-verify — no cargo available locally to confirm further.
1.78 also failed: a transitive dep (clap 4.6.1's Cargo.toml) requires the `edition2024` cargo feature, stabilized in Cargo 1.85 — unrelated to claude-tools' own code, just how new the pinned dependency tree is. No local cargo to bisect this by hand, and guessing one version per push-and-wait-5-minutes cycle is slow. `cargo msrv find` bisects the actual minimum compatible version in one run instead of asserting a guess. Once it reports the real number, Cargo.toml's rust-version will be set to match and the job switched back to `verify`.
Owner
Author
|
Closes #30. |
This was referenced Aug 10, 2026
BcKmini
added a commit
that referenced
this pull request
Aug 11, 2026
* fix: pre-existing bash 3.2 and Python syntax bugs found during testing - scripts/validate-agents.sh used `declare -A`, which crashes on bash 3.2 (the default /bin/bash on every stock macOS install, despite the repo's own "Platform: macOS" badge). Replaced with a portable space-delimited set so the script actually runs on macOS. - tools/claude-pipeline.py `list` used a backslash-escaped quote inside an f-string nested in another f-string — a SyntaxError on every Python version, so `claude-pipeline list`/`--help` never worked at all. Hoisted the value into a local variable instead. - Added __pycache__/*.pyc to .gitignore (generated while testing tools/). Found while verifying the autonomy/lessons/MCP-guide changes in this branch. * feat: add autonomy level (L0-L4) framework to harness design Harness type (tight/loose/adaptive) controls output constraint; autonomy level is a separate axis for how much human checking a task needs before or after the AI acts. Adds the 5-level model (L0 human-only .. L4 fully autonomous, L2 draft+review as the common default) from the AI Agent autonomy article this branch is based on. - autonomy: field added to all 11 agent frontmatters, assigned per role - claude-harness.py: new required check (autonomy declared), `autonomy` subcommand printing the L0-L4 table, templates updated - harness-designer (09): new design step + Autonomy Level output field - docs/HARNESS-GUIDE.md(.ko): new Autonomy Levels section - README Agent Roster table: new Autonomy column - /harness command + cheatsheets: autonomy validate check + prompts Closes #26 * feat: add claude-lessons failure/lessons-learned tool claude-handoff captures session state; nothing in this project recorded WHY something failed and HOW it was fixed, so the next session (or agent) had no way to avoid repeating a past mistake. Unlike handoffs, lessons accumulate indefinitely and are searchable by tag/keyword rather than pruned by age. claude-lessons add # symptom / root cause / fix / tags claude-lessons list # recent, optionally filtered by tag claude-lessons search Q # keyword search claude-lessons context # pipeable into claude for session-start context Stdlib-only, mirrors tools/claude-handoff.py conventions. Wired into Makefile install-tools/status/test-python and install.sh's TOOLS array. Closes #27 * feat: add MCP server guide with a working claude-lessons example Documents when to convert a CLI tool into an MCP server (Claude calls it mid-conversation) vs. keeping it a slash command or manual pipe (human stays in control of when it runs) — including a guideline against wrapping mutating/write actions as auto-callable MCP tools, tied to the autonomy levels added earlier in this branch. examples/mcp-lessons-server.py wraps tools/claude-lessons.py (add_lesson, search_lessons, recent_lessons) via the `mcp` Python SDK's FastMCP API. Lives under examples/, not tools/, since tools/ must stay dependency-free per docs/CONTRIBUTING.md — documented there as the one exception. Verified end-to-end against a real `mcp` install: `pip install mcp` now pulls a 2.x release that reworked/moved FastMCP, so the guide and example both pin `mcp>=1.2,<2`, confirmed working with 1.29.0. Closes #28 * docs: sync README.md/.ko.md for autonomy levels, claude-lessons, MCP guide - Agent Roster: new Autonomy column + explanation - Tools: 7 -> 8, new Tool 8 (claude-lessons) section, /lessons row, repo layout tree, context-cost-tips row - Nav bars + repo layout: MCP-GUIDE.md(.ko) link - README.ko.md also gets the harness/pipeline Tool 6/7 detail sections and full 11-agent repo layout it was missing — it had fallen out of sync with README.md (only the slash-command table and top badges had been updated when those tools were added), which this branch's changes would otherwise have made worse * Add tox for Python test/lint matrix + Rust MSRV verification (#31) * test: add pytest suite + tox for tools/*.py Nothing verified tools/*.py across Python versions before this — no pytest suite, no CI job, just a handful of ad-hoc --help/smoke commands in Makefile's test-python target. Adds: - tests/conftest.py: run_tool/home/git_repo fixtures — every test runs the real CLI as a subprocess against an isolated $HOME (and, for the two tools that shell out to git, an isolated git repo), so nothing touches the machine's real ~/.claude/ - tests/test_*.py: one file per tool, 44 tests total, functional not just --help (add/list/search/show roundtrips, error paths, exit codes) - tox.ini: py38-py313 (skip_missing_interpreters), lint (ruff check), fmt-check (ruff format --check) - pyproject.toml: scoped ruff config — E/F/I/UP only, not the full default ruleset (which includes bandit/blind-except/naive-datetime rules that fight this codebase's deliberate style), E402 ignored since every tool puts VERSION right after the docstring before imports - fmt-check / scripts/fmt.sh scoped to tools/claude-lessons.py + tests/ only, not all of tools/: the older tools use a deliberate hand-aligned style (aligned `=`, aligned dict values) that `ruff format` would flatten repo-wide — not forcing that as a side effect of adding tox - make tox: runs the full matrix (requires: pip install tox ruff) Verified locally: tox (py39/py312/py313 available here, py38/py310/py311 skip cleanly), lint, and fmt-check all pass. * fix: bugs found by the new pytest/tox matrix Real, independently-reproduced bugs the new test suite caught: - claude-cost.py: `set-budget` crashed with FileNotFoundError on a fresh install — `_save_budget` never created ~/.claude/ before writing cost-budget.json (snippet.py and claude-handoff.py already did this correctly; claude-cost.py's budget path was the one that didn't). - claude-remind.py / claude-review-diff.py: both use `X | None` (PEP 604) type annotations, which raise TypeError at import time on Python < 3.10 — every subcommand, including --help, was broken on 3.8/3.9 despite the "Python 3.8+" badge. Fixed with `from __future__ import annotations` (defers annotation evaluation, no behavior change). Confirmed via tox -e py39 before/after. Also applies the (small, low-risk) findings from the new ruff lint gate across tools/*.py: 4 unused imports, 1 dead local variable (claude-handoff.py's `list`, left over from a removed date column), 4 `if x: y` one-liners split to satisfy E701, and one ambiguous single- letter loop variable (`l` -> `line`). No behavior changes; tests still pass after each. * ci: add Python version matrix + Rust MSRV verification - python-tests job: runs `tox -e py` across Python 3.8-3.13 via actions/setup-python (this is what actually caught the claude-remind.py/claude-review-diff.py 3.8/3.9 breakage in the previous commit — nothing exercised those versions before) - python-lint job: `tox -e lint,fmt-check` - rust-version = "1.75" declared in claude-tools/Cargo.toml, matching the README's existing "Rust 1.75+" badge (previously just a claim, not enforced anywhere) - msrv job: `cargo msrv verify` — actually builds against the declared 1.75 toolchain instead of only ever testing against `stable` No cargo/rustc available in the environment this branch was authored in, so the Rust-side change (rust-version field + msrv CI job) is verified by CI on push, the same as the rest of this project's Rust checks. * docs: document tox/pytest and cargo-msrv workflow (EN/KO) README.md/.ko.md: Makefile section (make tox, make msrv), repo layout (tests/, tox.ini, pyproject.toml). Also fixes "make lint # clippy + ruff" in both READMEs — the lint target only ever ran clippy; ruff wasn't wired into it before this branch. docs/CONTRIBUTING.md/.ko.md: tox usage in Development Setup, tests/ step added to "Adding a New Tool", tox/cargo-msrv checks added to the PR checklist, cargo-msrv install/verify commands. Closes #30 * fix: MSRV verify failed — Cargo.lock is lockfile v4, needs cargo 1.78+ CI caught this immediately: cargo-msrv installs rustc 1.75 per the declared rust-version, but that toolchain's bundled cargo can't parse Cargo.lock's `version = 4` format (stabilized in cargo 1.78) — a structural incompatibility independent of whether the crate's actual code needs 1.75 or newer. The pre-existing "Rust 1.75+" badge this number was copied from was apparently never verified against the checked-in lockfile either. Bumped rust-version to 1.78 (README/README.ko badges updated to match) and letting CI's msrv job re-verify — no cargo available locally to confirm further. * ci: use cargo-msrv find instead of verify — real MSRV is unknown 1.78 also failed: a transitive dep (clap 4.6.1's Cargo.toml) requires the `edition2024` cargo feature, stabilized in Cargo 1.85 — unrelated to claude-tools' own code, just how new the pinned dependency tree is. No local cargo to bisect this by hand, and guessing one version per push-and-wait-5-minutes cycle is slow. `cargo msrv find` bisects the actual minimum compatible version in one run instead of asserting a guess. Once it reports the real number, Cargo.toml's rust-version will be set to match and the job switched back to `verify`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes #30. Stacked on #29 (
feat/autonomy-lessons-mcp) — base branch is set to that branch, notmain, since this adds tests forclaude-lessons.pywhich only exists there. Once #29 merges, GitHub will retarget this tomainautomatically.tox.ini+tests/: a real pytest suite for all 8 CLI tools (44 tests, subprocess-based against isolated$HOME/git repos — nothing touches the real~/.claude/), run across Python 3.8–3.13 via tox, pluslint/fmt-checkenvs.pyproject.toml: a ruff config scoped toE/F/I/UP— ruff's actual default ruleset turned out to be much broader (bandit-style security rules, blind-except, naive-datetime) and fighting this codebase's deliberate style (e.g. intentional broadexcept: passaround optional Windows VT-mode setup).E402is ignored since every tool putsVERSIONright after the docstring, before imports.fmt-checkis scoped totools/claude-lessons.py+tests/only — the older 7 tools use a deliberate hand-aligned style (aligned=, aligned dict values) thatruff formatwould flatten repo-wide. Asked in-session whether to force-reformat everything, format only new files, or drop the gate; went with formatting only new/touched-by-this-branch code and leaving the established style alone.scripts/fmt.shscoped the same way somake fmtdoesn't quietly undo it later.rust-version = "1.75"added toclaude-tools/Cargo.toml(matches the existing README badge, which wasn't enforced anywhere before), plus acargo msrv verifyCI job andmake msrvtarget.python-tests(matrix over 3.8–3.13) andpython-lintjobs in.github/workflows/ci.yml; newmsrvjob.Bugs the new tests actually caught (fixed in the second commit)
claude-cost.py set-budgetcrashed withFileNotFoundErroron a fresh install —_save_budgetnever created~/.claude/first.claude-remind.py/claude-review-diff.pyuseX | None(PEP 604) type annotations, which raiseTypeErrorat import time on Python < 3.10 — every subcommand of both tools, including--help, was broken on 3.8/3.9 despite the "Python 3.8+" badge. Fixed withfrom __future__ import annotations; confirmed viatox -e py39before/after (no py38/3.10/3.11 interpreter was available locally, but the failure mode is annotation-evaluation-at-import-time, identical on 3.8-3.9, and CI's matrix will confirm 3.8 directly).Plus the ruff lint gate's small, low-risk findings (4 unused imports, 1 dead variable, 4 one-line
ifstatements split for E701, onel->linerename) — all mechanical, tests re-verified green after each.Test plan
toxlocally (py39/py312/py313 available in this environment; py38/py310/py311 skip cleanly viaskip_missing_interpreters) — all green, including lint and fmt-checktox -e py39specifically re-run before/after thefrom __future__ import annotationsfix to confirm it was the actual causemake test(existing fast smoke suite) still passes after the lint cleanupclaude-harness check-all/bash scripts/validate-agents.shunaffected, re-run to confirmci.yml(parses, all expected jobs present)cargo msrv verify/ the newmsrvCI job — not run locally, no Rust toolchain available in this environment; will be exercised by CI on push, same as this project's other Rust checks