test(source-control): bind the wrapper-denial table in both directions - #1420
Conversation
`WRAPPER_DENIED_FLAGS` records the flags a `bin/` wrapper refuses before Python runs, and `test_wrapper_denied_flags_are_proven_by_a_refusal_row` already proved every LISTED flag is one a `bash-wrapper` refusal row invokes the wrapper to demonstrate. Nothing proved the converse: a new refusal row could demonstrate a second refused flag while the table stayed silent, leaving that flag spellable in a documented command. The table would be a subset of the wrapper's behavior while reading as a statement of it -- the exact unbacked-claim shape this contract exists to catch, pointed the other way. Read from `error_contains` rather than `argv`: a row's argv also carries the flags that reached the wrapper legitimately, while the error names the one the refusal is about. A companion assertion pins the premise the separate wrapper check rests on -- the merge parser does register `--allow-unpinned-head`, which is why a CLI-only check cannot see the refusal. If that stops holding the two checks have collapsed into one. Refs #1265. 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. |
|
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5d125e73b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Version only: main took 0.29.0 while this branch was open, so the patch slot moves 0.28.1 -> 0.29.1 and its changelog section is renamed to match. No content conflict; 387 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. |
The reverse binding read `error_contains` for `--`-prefixed tokens, but that field is a tuple of asserted output substrings with no invariant that any of them is a flag. A future bash-wrapper row with an empty tuple -- or one recording its rejected option without the leading dashes -- would yield nothing to iterate and pass vacuously, leaving WRAPPER_DENIED_FLAGS free to omit that refusal. The check would have reported success over exactly the omission it exists to catch. A wrapper refusal is ABOUT a flag, so a row that cannot name one cannot be bound: at least one extracted flag is now required per row. 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: #1420 landed 0.29.1 while this branch was open. This work keeps 0.30.0 and its section sits above the incoming 0.29.1. 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>
No linked issue — a gap found and closed while driving #1285 to merge, small enough that a tracking issue would outlive its usefulness before anyone read it.
Follow-up to #1285 (
#1265). Closes the reverse direction of a binding that #1285 established in one direction only.The gap
WRAPPER_DENIED_FLAGSrecords the flags abin/wrapper refuses before Python runs, so a documented wrapper command naming one is a command that always exits 2.test_wrapper_denied_flags_are_proven_by_a_refusal_rowproves every listed flag is one abash-wrapperrefusal row invokes the wrapper to demonstrate.Nothing proved the converse. A new bash-wrapper refusal row could demonstrate a second refused flag while the table stayed silent about it — and a document could then spell that flag unchallenged, because the documented-command check consults the table, not the rows. The table would be a subset of the wrapper's behavior while reading as a statement of it. That is the unbacked-claim shape this contract exists to catch, pointed the other way.
Changes
test_every_wrapper_refusal_row_reaches_the_denial_table— every flag a bash-wrapper refusal row names must be covered byWRAPPER_DENIED_FLAGS. Read fromerror_containsrather thanargv: a row's argv also carries the flags that reached the wrapper legitimately, while the error names the one the refusal is about.test_the_denied_flag_is_one_its_own_cli_accepts— pins the premise the separate wrapper check rests on. The merge parser does register--allow-unpinned-head, which is precisely why a CLI-only check cannot see the wrapper's refusal. If this stops holding, the two checks have collapsed into one and the narrowing is no longer load-bearing.Verification
python -m unittest tests.test_guards— 20 tests, OK. No production code changes; test and changelog only,source-control0.28.0 → 0.28.1.Related
WRAPPER_DENIED_FLAGSand the forward binding this PR completes--helpfix to_accepted_flagsin the same file; both touchtest_guards.pyand whichever lands second will need a trivial merge