fix(hooks): scope PR --base guard to the configured project repo#2014
Open
RapierCraft wants to merge 1 commit into
Open
fix(hooks): scope PR --base guard to the configured project repo#2014RapierCraft wants to merge 1 commit into
RapierCraft wants to merge 1 commit into
Conversation
The PreToolUse --base guard blocked every 'gh pr create --base main', regardless of which repository was targeted. It extracted --base but never --repo/-R, so it had no concept of repo scope. The rule exists to protect the project's own deploy pipeline, where main is the deploy trigger. Applied to a third-party repo it is a guaranteed false positive: an upstream awesome-list or docs fix typically has main as its only valid base and no staging branch at all, making legitimate external contribution impossible. Skip the guard when an explicit --repo/-R names a repo other than project.owner/project.repo from the cwd's forge.yaml. An undeterminable slug falls through to enforcement, so a malformed config cannot silently disarm the guard on the repo it protects. Adds 7 subprocess regression tests covering the external-repo exemption, the own-repo (explicit, case-insensitive, and implicit) still-blocked paths, the fail-closed undeterminable-slug case, and the commented-out repos: template block that forgedock init emits.
This was referenced Jul 16, 2026
Contributor
Author
3 tasks
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.
Problem
The PreToolUse hook's
--baseguard blocks everygh pr create --base main, regardless of which repository is targeted.checkPrTarget()extracts--basebut never--repo/-R, so it has no notion of repo scope and governs every repo on the machine.The rule exists to protect this project's deploy pipeline, where
mainis the deploy trigger. Applied to a third-party repo it is a guaranteed false positive — an upstream awesome-list or a docs typo fix typically hasmainas its only valid base and nostagingbranch at all. The result: legitimate external contribution is impossible from any ForgeDock-managed directory.Found in the wild: an AlterLab issue whose entire remaining work was a one-line PR to
punkpeye/awesome-mcp-serverssat stranded atneeds-human, with its own devdoc recording the cause — "blocked by this repo's own main-branch pipeline guardrail (which hard-blocks any--base main, including on unrelated external repos)". Several sibling registry-submission issues are stuck the same way.This is invisible to ForgeDock's own maintainers because this repo's worktree has no
forge.yaml(gitignored), soisForgeDockManagedCwd()returns false and the hook self-exempts.Fix
Skip the guard when an explicit
--repo/-Rnames a repo other thanproject.owner/project.repofrom the cwd'sforge.yaml.Deliberately fail-closed. An undeterminable slug (no
forge.yaml, or noproject.owner/repoin it) falls through to enforcement rather than skipping it, so a malformed config can never silently disarm the guard on the repo it is meant to protect. Only a positive mismatch — an explicit-Rnaming a repo demonstrably not ours — is exempted.Slug reading is a line-anchored regex, not a YAML parse: this hook runs on every tool call and must stay synchronous and dependency-free. The
^\s*owner:anchor cannot match a commented# owner:line, so the commentedrepos:template blockforgedock initemits is ignored (covered by a test).Verification
node --test bin/tests/pre-tool-use.test.mjs→ 57/57 pass (50 pre-existing + 7 new, no regressions).Verified against a real managed project (
rapiercraftstudios/alterlab):-R punkpeye/awesome-mcp-servers --base main-R RapierCraftStudios/AlterLab --base main--base main(implicit own repo)-R RapierCraftStudios/AlterLab --base stagingThis PR is itself end-to-end proof: opening it required the patched hook, since the stock hook blocks
--base mainon this repo when invoked from a managed cwd.Tests added
7 subprocess regression tests (against the real hook via
runHook(), not the duplicated pure-logic copy):--repolong form and-Rshort form-R)repos:template block does not poison slug parsingNote for maintainers
bin/tests/pre-tool-use.test.mjskeeps its own duplicate copy ofcheckPrTargetfor the "pure logic"describeblock, so those tests exercise a copy that can drift from the real hook. I left it alone and put all new coverage in therunHook()subprocess block, which tests the real file — but the duplication looks worth collapsing separately.