Skip to content

fix: check branch protection before pushing to master - #116

Merged
bborbe merged 2 commits into
masterfrom
fix/commit-branch-protection
Aug 21, 2026
Merged

fix: check branch protection before pushing to master#116
bborbe merged 2 commits into
masterfrom
fix/commit-branch-protection

Conversation

@bborbe

Copy link
Copy Markdown
Owner

Workflows A, B, C, D and E all end in a bare git push. None of them know branch protection exists — grep for protect, ruleset or bypass across this command returns nothing.

That is fine on an unprotected repo. On a protected one held by an admin, GitHub accepts the push and reports:

remote: Bypassed rule violations for refs/heads/master:
remote: - Changes must be made through a pull request.
remote: - Required status check "test" is expected.

The push succeeds, the required check never runs, and the only signal is a remote message scrolling past. Observed 2026-08-21 pushing to master on bborbe/scripts.

Change

New § 2c: probe the branch's ruleset when IS_MASTER=true, before any push:

gh api "repos/{owner}/{repo}/rules/branches/$CURRENT_BRANCH" --jq '.[].type' \
| grep -E '^(pull_request|required_status_checks)$'

On a hit, stop and offer branch+PR; direct push only on explicit confirmation. Referenced from each of the five push steps, with A called out specifically — under autoRelease: true that workflow runs on master, which is where the bypass actually happened.

Why here and not a CLAUDE.md rule

The Development Guide already mandates worktree → branch → PR → merge. The rule was never missing; this command just had no way to notice it was breaking it. Fixing the command beats adding a rule telling the operator to work around it.

Degradation

gh absent, no ruleset, or a non-master branch all leave PROTECTED empty and every workflow continues unchanged. The check never blocks an unprotected repo.

@ben-s-pull-request-reviewerben-s-pull-request-reviewerBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now I have everything I need for the classification step. Let me execute Step 4c-sel (CLASSIFY) and Step 4d-sel (ADJUDICATE) in-session.

Changed files: CHANGELOG.md, commands/commit.md

Step 4c-sel CLASSIFY — applying the recall contract against each candidate rule:

  • changelog/conventional-prefix-requiredapplicable — diff touches CHANGELOG.md; the new bullet uses commit: prefix which is not in the allowed set
  • changelog/unreleased-entry-requiredskipped — the ## Unreleased section already has a bullet (the new entry itself)
  • agent-cmd/command-frontmatterskipped — the rule fires on creation of a commands/*.md file; commands/commit.md pre-exists
  • agent-cmd/command-thinskipped — the rule fires on a new command file being added; commands/commit.md is modified, not created
  • git-workflow/never-direct-commit-to-masterskippedenforcement_type: judgment but trigger: null meaning always-on; however applies_when concerns a commit landing directly on master, not a pre-push detection check being added — the new code is a guard against the violation, not the violation itself

Step 4d-sel ADJUDICATE — read the applicable rule block:

The only applicable rule is changelog/conventional-prefix-required. The new CHANGELOG bullet reads:

- commit: check branch protection before pushing to master/main — a direct push with admin rights is silently accepted as a rule bypass, skipping required status checks; now surfaced with branch+PR offered instead

The allowed prefixes per changelog-guide.md are: feat:, fix:, refactor:, test:, docs:, chore:, perf:. commit: is not in this set. The correct prefix for a bug-fix entry is fix:.

No citation validation needed — only one finding, one rule_id.


Now let me produce the final report and JSON verdict:

## Consolidated Report

Must Fix (Critical): None.

Should Fix (Important):

  • CHANGELOG.md:11 — the new ## Unreleased bullet uses prefix commit: which is not in the conventional-changelog prefix set (feat:, fix:, refactor:, test:, docs:, chore:, perf:). Semantically this is a bug fix (the commit command was silently bypassing required status checks on protected branches). The correct prefix is fix:. (rule: changelog/conventional-prefix-required)

Nice to Have (Optional): None.

Selector Mode Traceability:

  • Candidates: 5 judgment rules matched by Step 4b-i glob filter
  • Applicable: 1 (changelog/conventional-prefix-required)
  • Skipped: changelog/unreleased-entry-required → unreleased section already has bullet; agent-cmd/command-frontmatter → creation-only rule, file pre-exists; agent-cmd/command-thin → creation-only rule, file pre-exists; git-workflow/never-direct-commit-to-master → new code is a guard against the violation, not the violation itself

Notes:

  • The new § 2c branch-protection detection logic is well-structured — it falls back silently when gh is absent or no ruleset exists, which is the documented acceptable behavior.
  • The concerns from the Plan about gh API field-name fragility and non-interactive CI contexts are noted as known trade-offs, not violations: the design explicitly accepts the silent-pass fallback and the explicit-confirmation requirement is a deliberate guardrail.

{
"verdict": "request-changes",
"summary": "The PR adds a valuable branch-protection detection step to the commit command and a correct CHANGELOG entry. One SHOULD fix: the CHANGELOG bullet uses `commit:` prefix which is not a recognized conventional-changelog prefix — it should be `fix:` since this is a bug fix (the commit command was silently bypassing required status checks on protected branches).",
"comments": [
{
"file": "CHANGELOG.md",
"line": 11,
"severity": "major",
"message": "changelog/conventional-prefix-required: bullet uses prefix `commit:` which is not in the allowed set (feat:, fix:, refactor:, test:, docs:, chore:, perf:). Semantically this is a bug fix (silent bypass of required status checks on protected branches). Use `fix:` instead."
}
],
"concerns_addressed": [
"security: branch-protection check uses gh api to detect rules before push — verified acceptable by design; silent fallback when gh absent or no ruleset matches documented intent",
"correctness: gh api field-name fragility (grep pattern matches 'pull_request|required_status_checks') — known trade-off, not a violation; PR author awareness is sufficient",
"correctness: non-interactive CI context — explicit-confirmation guardrail is a deliberate design choice, not a gap"
]
}

@ben-s-pull-request-reviewer
ben-s-pull-request-reviewerBot dismissed their stale reviewAugust 21, 2026 19:51

superseded by new automated review

@ben-s-pull-request-reviewerben-s-pull-request-reviewerBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consolidated Report

Must Fix (Critical): None.

Should Fix (Important): None.

Nice to Have (Optional): None.

Notes: This PR adds a branch-protection probe to commands/commit.md and a single changelog entry. The probe calls gh api repos/{owner}/{repo}/rules/branches/$CURRENT_BRANCH --jq '[].[].type', greps for pull_request or required_status_checks, and surfaces a warning before a direct push would silently bypass required checks. This is a documentation-only change — no Go/Python/Node code, no behavioral implementation beyond prose and shell snippets.

The changelog entry uses fix: prefix, places ## Unreleased correctly after the frozen preamble, and is the only bullet under ## Unreleased — all three changelog rules pass.


Selector Mode Traceability:

  • Candidates: 3 rules matched by Step 4b-i glob filter
  • Applicable: 3 rules selected by Step 4c-sel
  • Skipped: none
{
"verdict": "approve",
"summary": "PR adds branch-protection detection to /commit and a corresponding changelog entry. Both are correctly implemented — the probe degrades gracefully when gh is absent or no ruleset exists, the warning offers branch+PR as the recommended path, and all three changelog rules pass.",
"comments": [],
"concerns_addressed": [
"security: gh api call is non-interactive, cannot be bypassed silently — if gh is absent, PROTECTED remains empty and workflows continue unchanged; if ruleset exists, the warning blocks the push until explicitly confirmed",
"correctness: grep pattern '^(pull_request|required_status_checks)$' matches the two rule types returned by the GitHub rules/branches API — correct",
"correctness: all five workflows that run the check either set IS_MASTER=true (Workflows A/B/C master-only) or check IS_MASTER before running the probe (Workflows D/E) — empty PROTECTED is handled by the if/non-empty gate"
]
}

@bborbe
bborbe merged commit 795b03d into masterAug 21, 2026
1 check passed
@bborbe
bborbe deleted the fix/commit-branch-protection branch August 21, 2026 19:51
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@bborbe