fix(hooks): resolve the interpreter once by presence, never a python3-X-or-python-X fold (#577) - #614
Merged
Merged
Conversation
…-X-or-python-X fold (#577) Every command/skill surface that spelled the cross-host interpreter fallback as `python3 X || python X` branched on the helper's exit code, so any helper whose exit code carries information (e.g. `taskwrite.py archive`'s "archive could not be read", exit 3) got silently re-run under `python` on ANY nonzero exit, and the caller saw only the second run's verdict. Already fixed in the `release` skill (#576); this closes the rest of the repo. Fixed the fold in core/surface/commands/{doctor,metrics,override,pr, preview,prune,standup,task,watch}.md, core/surface/includes/ security-gate-record.md, and core/surface/skills/{commit-gate, tribunal}/SKILL.md, plus the matching "Invoked by ... as" header comments in core/pysrc/{babysit,metrics,migration-pass,preview, security-pass,taskwrite}.py. Every site now resolves once via `PY=python3; { command -v python3 >/dev/null 2>&1 && python3 --version >/dev/null 2>&1; } || PY=python` and invokes `"$PY"`. plugins/*/hooks/hooks.json is unchanged by design (#577's own carve-out): its PreToolUse/SessionStart entries are pass/fail with no exit-code vocabulary to lose. Regenerated all three plugin trees via `tools/build-surface.py` and `tools/sync-core.py` (both --check clean). Version advance: ca 2.11.8, ca-codex 0.4.7, ca-pi 0.2.7, root package.json regenerated, README badge/ships-line updated, CHANGELOG + ca-pi CHANGELOG sections added. CHANGELOG: Command/skill surfaces resolve the Python interpreter once by presence instead of a python3-or-python exit-code fold that could discard a helper's real verdict. Claude-Session: https://claude.ai/code/session_01QjJeSbcwPHwMmd6CEZeagB
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) 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:
Comment |
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
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
The
python3 X || python Xcross-host interpreter convention branches on thehelper's EXIT CODE, so it cannot distinguish "no
python3" from "the helperran and told you something." Any helper whose exit code carries information
(not just 0/non-zero) gets silently re-run under
pythonon any nonzeroexit, and the caller sees only the second run's verdict — already fixed in
release/SKILL.mdfor #576; this closes the rest of the repo per #577.Every remaining site now resolves the interpreter ONCE, by presence, before
invoking a helper:
Call-site inventory
Fixed (core/surface, canonical — regenerated into all three plugin trees via
tools/build-surface.py):commands/doctor.md—doctor.pycommands/metrics.md—metrics.py(x2 call sites + prose)commands/override.md—security-pass.py(Security ceiling heavier path)commands/pr.md—babysit.pycommands/preview.md—preview.py(diffandsecretsmodes)commands/prune.md—prune-transcript.py(x2)commands/standup.md—boardsync.py reconcile,taskwrite.py archive(the two named in Thepython3 X || python Xinterpreter convention masks helper exit codes repo-wide #577's own table)commands/task.md—taskwrite.py(also normalizedstart/done, which had no fallback at all before, to the same"$PY"spelling for internal consistency)commands/watch.md—babysit.pyincludes/security-gate-record.md—security-pass.py(shared bycrypto-compliance/secret-handling, which reference it rather than duplicating the command)skills/commit-gate/SKILL.md—migration-pass.pyskills/tribunal/SKILL.md—tribunal-usage.py observe(Claude-only leg,${CLAUDE_PLUGIN_ROOT})Also updated the matching "Invoked by ... as" header-comment examples in
core/pysrc/{babysit,metrics,migration-pass,preview,security-pass, taskwrite}.pyso the documented invocation contract stays consistent withthe command prose that actually calls them.
Deliberately kept —
plugins/*/hooks/hooks.json'sPreToolUse/SessionStartdual registration (python -c "..." || python3 "<script>").Per #577's own carve-out: these hooks are pass/fail with no exit-code
vocabulary to lose, so the fold's information-loss defect doesn't apply.
docs/hooks.mdcorrectly describes this hooks.json behavior and needed nochange.
_hooklib.py/_githooks.pyalready carry comments explaining whythe git-level shim and hooks.json avoid the fold — explanatory, not
instances of it.
Reported, not fixed —
plugins/ca-pi/tools/src/bridge.ts(
resolvePythonCommand). This already resolves-by-presence correctly: itprobes each interpreter candidate directly via a dedicated
spawnSync(..., ["-c", "import sys; ..."])call, entirely separate from invoking the actualhelper, so there's no shared exit code to conflate. Not an instance of the
#577defect; no change needed.Out of scope (bare
python3 Xwith no fallback at all, a differentdefect than the
||fold this issue targets):skills/context-creation/ SKILL.mdandskills/debug/SKILL.md'staskwrite.py addcalls.Version advance
ca2.11.7 → 2.11.8,ca-codex0.4.6 → 0.4.7,ca-pi0.2.6 → 0.2.7, rootpackage.jsonregenerated, README badge + ships-line updated, CHANGELOG.mdand
plugins/ca-pi/CHANGELOG.mdsections added.Test results
python tools/build-surface.py --check→ OK (claude, codex, pi in sync)python tools/sync-core.py --check→ OK (56 core files x 3 plugins, byte-identical)git diff --check→ cleanenv -u NO_COLOR python -m pytest plugins/ca/hooks/tests -q→ 1301 passed,130 subtests passed, 10 failed — all 10 are the pre-existing Worktree sessions: security-pass.py and H-09b/H-10b resolve different project roots — gate pass unrecordable #604
worktree-root family (
test_git_hooks.py/test_repo_resolution.py,"leaving the shared enforcer entry as it is... Start a session from the
main checkout to refresh it"), reproduced identically on a stashed clean
origin/mainin the same worktree; this diff touches no git-hook file.python -m pytest .github/scripts/test_build_surface.py -q→ 40 passedpython .github/scripts/test_routing_and_cleanup_surface.py→ 19 passed (OK)python .github/scripts/check_badge_consistency.py→ consistentpython .github/scripts/test_release_trace.py→ 29 passedpython .github/scripts/payload_version_gate.py --plugin plugins/ca --base origin/main→ 2.11.7 -> 2.11.8python .github/scripts/payload_version_gate.py --plugin plugins/ca-codex --base origin/main→ 0.4.6 -> 0.4.7python tools/build-host-packages.py --check --release-guard-base origin/main→ Pi payload/version/changelog/root metadata advanced together: 0.2.6 -> 0.2.7python .github/scripts/check_site_voice.py→ 39 authored pages cleanNo new tests were added — this is a prose/documentation and doc-comment fix
with no new executable code path, so the mutation-proof requirement (every
NEW test dies to a mutant) doesn't apply; the existing suites above cover
the unchanged executable surfaces.
Parity watch
No unexplained ca/ca-codex/ca-pi divergence — the fix is generated
identically into all three trees from the same core/surface source, and the
Claude-only
tribunal-usage.pyleg is the one place${CLAUDE_PLUGIN_ROOT}(shell-resolved) is used instead of
{{PLUGIN_ROOT}}(build-time token),which is pre-existing and unrelated to this change.
Closes #577
https://claude.ai/code/session_01QjJeSbcwPHwMmd6CEZeagB