fix(hook-kit): scope PR-URL bare-ref check to per-number match, allow force-push in worktrees - #374
Draft
daegunjhy wants to merge 1 commit into
Draft
fix(hook-kit): scope PR-URL bare-ref check to per-number match, allow force-push in worktrees#374daegunjhy wants to merge 1 commit into
daegunjhy wants to merge 1 commit into
Conversation
… force-push in worktrees check_ask_bare_ref previously joined the whole AskUserQuestion payload into one blob before checking for a URL, so a URL for one PR/issue number silently excused every OTHER option's bare "#N" reference in the same ask. Reproduced live: an option citing #13/#23/#17 with no URL passed because a different option in the same ask happened to cite a URL for #22/#21 (failed-attempts.md class=ask-option-pr-ref-missing-url, 4th recurrence). Rewrote the check to compare the set of numbers referenced via bare "#N" against the set of numbers covered by a full PR/issue URL anywhere in the payload, so a URL only excuses bare refs to that same number. An initial per-text-unit attempt was discarded after its own false-positive pass caught it wrongly denying the common "question names the PR, an option's description carries the URL" pattern. Also allow a force push when BOTH are positively confirmed: the target checkout is a git worktree (git-dir differs from git-common-dir) and the resolved branch is not main/master. Any failed check falls back to the prior unconditional block (fail closed). Verified: self-test suite 93/93 passed; 5 false-positive samples passed; the reproduced violating payload is blocked with a precise missing-number list; 3 real scenarios (worktree+feature=allow, worktree+main=block, non-worktree=block) behave as intended. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
fix(hook-kit): scope PR-URL bare-ref check to per-number match, allow force-push in worktrees
check_ask_bare_ref previously joined the whole AskUserQuestion payload into
one blob before checking for a URL, so a URL for one PR/issue number silently
excused every OTHER option's bare "#N" reference in the same ask. Reproduced
live: an option citing #13/#23/#17 with no URL passed because a different
option in the same ask happened to cite a URL for #22/#21 (failed-attempts.md
class=ask-option-pr-ref-missing-url, 4th recurrence). Rewrote the check to
compare the set of numbers referenced via bare "#N" against the set of
numbers covered by a full PR/issue URL anywhere in the payload, so a URL
only excuses bare refs to that same number. An initial per-text-unit attempt
was discarded after its own false-positive pass caught it wrongly denying
the common "question names the PR, an option's description carries the URL"
pattern.
Also allow a force push when BOTH are positively confirmed: the target
checkout is a git worktree (git-dir differs from git-common-dir) and the
resolved branch is not main/master. Any failed check falls back to the
prior unconditional block (fail closed).
Verified: self-test suite 93/93 passed; 5 false-positive samples passed;
the reproduced violating payload is blocked with a precise missing-number
list; 3 real scenarios (worktree+feature=allow, worktree+main=block,
non-worktree=block) behave as intended.
Co-Authored-By: Claude Sonnet 5 noreply@anthropic.com