refactor(disk-hygiene): derive the Python version floor from one origin

[kyle-sexton]

Summary

Fixes finding 3 (SSOT drift) from handoff-inbox item 20260722-004536-disk-hygiene-setup-audit: the "Python 3.11+" floor was hand-maintained in ≥5 places — hygiene.py's sys.version_info < (3, 11) runtime check (the real enforcement), both .test.sh wrappers, setup/SKILL.md (twice), clean/SKILL.md, and the README — while the setup skill told itself "probe what they actually require, don't recite this file". A future floor bump would drift the copies silently.

• hygiene.py gains a module-level MIN_PYTHON = (3, 11) constant; the runtime check and its error message derive from it.
• New VersionFloorTests lock the constant's greppable line shape (^MIN_PYTHON = (\d+, \d+)$) and prove enforcement + error text use it (red-first).
• Both .test.sh wrappers parse the constant via sed instead of restating the number, and fail loudly if the parse breaks.
• setup check step 1 now derives the probed floor from the constant; apply guidance points at the parsed floor.
• Remaining prose (clean/SKILL.md gotchas, README requirements) reworded to pointers or explicitly-annotated convenience copies. API-fact mentions of Python 3.11 (e.g. "3.11 has no os.path.isjunction") are left alone — they are not floor copies.
• Version 0.6.0 → 0.6.1 + CHANGELOG.

Testing

• test_hygiene.py: 95 tests OK (4 platform skips), including the 3 new floor tests.
• Both wrappers run green with the parsed floor; parse-failure path exits 1 with a named FAIL.
• check-changed-skills.sh PASS, check-changelog-parity.sh --check-bump PASS, shellcheck clean on both wrappers.

Related

• Closes refactor(disk-hygiene): derive the Python version floor from one origin #1008
• Handoff-inbox item: 20260722-004536-disk-hygiene-setup-audit (F3, MED)

🤖 Generated with Claude Code

[claude]

Claude finished @kyle-sexton's task in 2m 37s —— View job

---

PR Review

• Gather context and read changed files
• Review hygiene.py — constant definition and runtime enforcement
• Review test_hygiene.py — new VersionFloorTests
• Review .test.sh wrappers — sed-based floor parsing
• Review setup/SKILL.md and clean/SKILL.md — prose changes
• Review plugin.json, CHANGELOG.md, README.md
• Synthesize findings and post review

---

The approach is sound — a single MIN_PYTHON constant as the canonical source, runtime enforcement derived from it, both shell wrappers parsing it, docs annotated as pointers or convenience copies. Two minor issues worth addressing before merge, both in the tests.

---

Finding 1 — Test name implies a check that isn't there

test_hygiene.py line 1037

def test_engine_enforces_the_constant_and_names_it_in_the_error(self) -> None:
    ...
    self.assertNotEqual(0, code)   # ← only checks exit code

The name says "names it in the error" but the body only asserts a non-zero exit code. The error-message content check lives in test_error_message_derives_from_the_constant. Either rename this to test_engine_enforces_the_constant (removing the "names it in the error" claim), or add an assertion that the floor string appears in the output alongside the exit-code check. The current split means the name is misleading and a reader can't tell what the two tests actually cover at a glance.

Fix this →

---

Finding 2 — below breaks if MIN_PYTHON minor is ever 0

test_hygiene.py line 1038, line 1050

below = (hygiene.MIN_PYTHON[0], hygiene.MIN_PYTHON[1] - 1, 0)

Today MIN_PYTHON = (3, 11) so below = (3, 10, 0) — fine. But if the floor is ever bumped to a new major ((4, 0)), this produces (4, -1, 0), which is not a valid Python version tuple and could cause surprising mock behaviour. A more robust approach:

# one full minor below, staying within the same major
if hygiene.MIN_PYTHON[1] > 0:
    below = (hygiene.MIN_PYTHON[0], hygiene.MIN_PYTHON[1] - 1, 0)
else:
    below = (hygiene.MIN_PYTHON[0] - 1, 99, 0)

Or simply use a hard-coded tuple that is guaranteed to be ancient and always below any realistic floor ((2, 7, 0)), with a comment explaining the intent. This is a latent issue rather than a current bug, but the test is explicitly intended to survive a floor bump.

Fix this →

---

Everything else looks good

• hygiene.py: MIN_PYTHON = (3, 11) placed correctly before other module-level constants; runtime check and f-string error both derive from it cleanly. ✓
• test_min_python_line_keeps_its_greppable_shape: The regex r"^MIN_PYTHON = \((\d+), (\d+)\)$" with re.MULTILINE correctly locks the exact line shape the sed commands and grep instruction depend on; the assertEqual(1, len(matches)) also catches accidental duplication. ✓
• sed patterns: sed -n 's/^MIN_PYTHON = (\([0-9]*\), \([0-9]*\)).*/\1.\2/p' is valid POSIX BRE and portable to both GNU and BSD sed. The [0-9]*-only capture makes the $FLOOR value safe for interpolation into the Python -c string. ✓
• kill_switch_probe.test.sh: The relative ENGINE path ($SCRIPT_DIR/../../clean/scripts/hygiene.py) is structurally correct for the plugin layout and stays within plugin boundaries. ✓
• setup/SKILL.md step 1: The instruction to parse MIN_PYTHON from ${CLAUDE_PLUGIN_ROOT}/skills/clean/scripts/hygiene.py rather than reciting a number is exactly right for an AI-executed step. ✓
• clean/SKILL.md: Both updated paragraphs (arguments/boundaries and gotchas) correctly refer to MIN_PYTHON as the floor's single origin without restating the number. ✓
• README.md: The parenthetical annotation (the floor's single origin is the \MIN_PYTHON` constant … treat the number printed here as a convenience copy)` is clear and accurate. ✓
• Version bump 0.6.0 → 0.6.1: Appropriate; this is a no-behavior-change refactor. ✓
• CHANGELOG: The entry accurately describes what changed and why. ✓

[claude]

Claude finished @kyle-sexton's task in 2m 35s —— View job

---

Security Review — PR #1028

• Gather context and understand the request
• Fetch PR diff
• Review shell scripts for injection / unsafe patterns
• Review Python changes for security issues
• Review skill/doc changes for information exposure
• Post findings

---

No exploitable security vulnerabilities found. The changes are a refactor (SSOT extraction) with test coverage; the security surface is small and handled defensively. Full analysis below.

---

Shell injection in $FLOOR — examined, not exploitable

Both wrapper scripts embed $FLOOR inside a Python -c one-liner via a bash double-quoted string:

# hygiene.test.sh:24 / kill_switch_probe.test.sh:25
"$PYTHON" -c "import sys; floor = tuple(int(part) for part in '$FLOOR'.split('.')); raise SystemExit(0 if sys.version_info >= floor else 1)"

$FLOOR is assigned from sed capture groups that match only [0-9]*:

FLOOR="$(sed -n 's/^MIN_PYTHON = (\([0-9]*\), \([0-9]*\)).*/\1.\2/p' "$SCRIPT_DIR/hygiene.py")"

\1 and \2 can only contain decimal digits; the substitution joins them with a literal .. The resulting value is therefore always of the form <digits>.<digits> — no shell metacharacters, no quotes, no $, no spaces. No injection is possible.

An empty-match edge case (MIN_PYTHON = (, ) in the source) would yield FLOOR=".", causing int('') to raise ValueError inside Python and exit non-zero — the SKIP branch triggers. The scripts never proceed with a bad parse.

Confidence: HIGH — restriction is structurally enforced by the sed regex, not by trust in file contents.

Cross-directory path in kill_switch_probe.test.sh — examined, safe

ENGINE="$SCRIPT_DIR/../../clean/scripts/hygiene.py"

The ../.. traversal is intentional: the setup script is at setup/scripts/, so this resolves to clean/scripts/ — both sibling directories under the same plugin root. SCRIPT_DIR is set with cd "$(dirname "${BASH_SOURCE[0]}")" && pwd, which produces a canonical absolute path via pwd. If the file is absent or the path resolves unexpectedly, sed produces empty output → FLOOR is empty → the script exits 1 with a named FAIL before any Python is invoked. Fail-closed.

Python MIN_PYTHON extraction — no risk

hygiene.py lines 23 and 1575–1577:

MIN_PYTHON = (3, 11)
...
floor = ".".join(str(part) for part in MIN_PYTHON)
raise HygieneError(f"disk-hygiene requires Python {floor} or newer")

MIN_PYTHON is a tuple of integer literals. str(int) produces only digits. The formatted error string carries no user-controlled input. No injection surface.

Test-suite mocking — safe

test_hygiene.py lines 1038, 1050:

below = (hygiene.MIN_PYTHON[0], hygiene.MIN_PYTHON[1] - 1, 0)
mock.patch.object(hygiene.sys, "version_info", below)

mock.patch.object replaces an attribute in-process for the duration of a with block; no shell execution, no subprocess, no file writes. The tuple arithmetic is bounded by the constant's own values (currently (3, 10, 0)).

SKILL.md grep instruction — informational, not executable

The updated setup/SKILL.md tells the AI agent to run grep -m1 '^MIN_PYTHON' … to derive the floor. This is prose instruction for an AI, not a shell script. The agent uses the parsed value only to probe the interpreter version — it does not interpolate the raw string into a shell command. No actionable injection surface.

No workflow changes, no secrets handling, no egress changes

No .github/workflows/ files were modified. The PR introduces no new network calls, no credential handling, no permission changes, and no new external dependencies.

---

Summary: This is a clean SSOT refactor. The one pattern that warrants scrutiny — embedding a parsed file value into a shell-launched Python one-liner — is safe by construction: the sed regex constrains the value to [0-9]+\.[0-9]+ before it ever reaches the command string.