fix(source-control): refuse flag abbreviation on every babysit entry point#1428
Merged
Conversation
…point Closes #1371. A permission grant states its condition as the literal presence or absence of a flag in the command text -- above all "no --merge means check-only". Argparse's default prefix abbreviation lets `--mer` resolve to `--merge` while the text contains no such flag, so the written command and the resolved behavior diverge, which is exactly what such a condition has to be able to rule out. #1354 closed this on babysit_merge.py and babysit_resolve_thread.py. The other seven entry points still inherited the default: babysit_findings.py, manage_babysit_lease.py, manage_feedback_ledger.py, pr_queue_snapshot.py, prune_babysit_worktrees.py, refresh_pr_branch.py, request_review.py. All nine now set allow_abbrev=False. Hardening them one at a time is what let the gap persist for seven files, so the property is bound to the directory instead: the guard contract gains a gate that invokes every catalogued Python entry point with `--hel` and requires a nonzero exit. `--help` is registered on every parser and short-circuits parsing, so an abbreviation that resolves exits 0 before required-argument validation runs, while one that does not is a usage error -- which makes the exit code sufficient without a per-CLI argument shape. The message is deliberately not asserted: several of these parsers have a required mutually exclusive group that errors before any unrecognized argument is reported. A companion test asserts that discrimination against argparse itself rather than assuming it, and the gate was verified by reverting one parser: it fails naming that file, and passes again when restored. Abbreviated invocations that previously worked are now usage errors, which is the intent; version takes a minor bump for it. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
8 tasks
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Warning Automated review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
|
Warning Automated security review did not complete — this is an infrastructure failure, not a review verdict. Treat any Claude comment on this PR (including a placeholder like "I'll analyze this and get back to you") as incomplete, not "no findings."
Re-running the job, or pushing a new commit, will retry the review. |
… of the spell-checker `--hel` is a typos hit (`hel` -> `help`/`hell`/`heal`), and `_typos.toml` is synced verbatim from the root canonical policy, so a repo-specific allow entry there would not survive. The probe is now a single named constant carrying the blessed `spellchecker:disable-line` directive, referenced everywhere else; the prose and the changelog describe it as an unambiguous three-character prefix of `--help` instead of spelling it. Recorded while doing so: three characters is load-bearing, not incidental. `manage_babysit_lease.py` also registers `--heartbeat-interval-seconds`, so a one- or two-character prefix is AMBIGUOUS there and exits 2 with abbreviation still on -- the probe would have passed on that entry point while proving nothing. typos clean; 387 tests pass. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Version and changelog ordering only: #1420 landed 0.29.1 while this branch was open. This work keeps 0.31.0 and its section sits above the incoming 0.29.1. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
Version and changelog ordering only: #1264 landed 0.30.0 while this branch was open. This work keeps 0.31.0 and its section sits above the incoming 0.30.0. 392 python tests and 135 gate tests pass. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
|
Claude encountered an error after 2s —— View job I'll analyze this and get back to you. |
Contributor
Author
|
. |
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.
Closes #1371.
Replaces #1382, which is closed in favour of this. #1382 was opened before #1354 landed and is now a strict superset of it: re-scoped here to the residual only, on a fresh base, with the property bound to the directory rather than to a list of files.
The gap
A permission grant states its condition as the literal presence or absence of a flag in the command text — above all "no
--mergemeans check-only". Argparse's default prefix abbreviation lets--merresolve to--mergewhile the command text contains no such flag, so the written command and the resolved behavior diverge. That is exactly what such a condition has to be able to rule out.#1354 closed this on
babysit_merge.pyandbabysit_resolve_thread.py. Seven entry points still inherited the default:babysit_findings.py·manage_babysit_lease.py·manage_feedback_ledger.py·pr_queue_snapshot.py·prune_babysit_worktrees.py·refresh_pr_branch.py·request_review.pyAll nine now set
allow_abbrev=False.Why a gate, not seven more edits
Hardening entry points one at a time is what let the gap survive #1354 for seven files. The guard contract gains a check over the whole catalogue: every catalogued Python entry point is invoked with
--heland must not exit 0.--helpis registered on every parser and short-circuits parsing, so an abbreviation that resolves exits 0 before required-argument validation ever runs, while one that does not is a usage error. That makes the exit code a sufficient discriminator without a per-CLI argument shape — which is what makes this a gate over the catalogue rather than a hand-maintained list of cases. The message is deliberately not asserted: several of these parsers have a required mutually exclusive group that errors before any unrecognized argument is reported.A companion test asserts that discrimination against argparse itself rather than assuming it.
Verification
python -m unittest discover -s tests— 387 tests, OK.request_review.py'sallow_abbrev=Falsefails the gate naming that file (resolved the abbreviation --hel to --help and exited 0), and it passes again on restore.Compatibility
Abbreviated invocations that previously worked are now usage errors. That is the intent, and the version takes a minor bump for it —
source-control0.29.0 → 0.31.0 (0.30.0 is claimed by #1264, open).Related