Skip to content

Claude Code: prompt before git/gh write operations and PR instructions#1377

Open
labkey-martyp wants to merge 7 commits into
developfrom
fb_claude_git_confirm
Open

Claude Code: prompt before git/gh write operations and PR instructions#1377
labkey-martyp wants to merge 7 commits into
developfrom
fb_claude_git_confirm

Conversation

@labkey-martyp
Copy link
Copy Markdown
Contributor

@labkey-martyp labkey-martyp commented May 15, 2026

Rationale

When working with the Claude Code agent, certain git and gh commands should be confirmed by the user. This branch makes the agent surface a confirmation prompt before any of those, instead of executing silently.

Additionally, Claude creating PRs should follow our established templates and creating feature branches should follow our feature branch naming convention.

Why both a hook and permissions.ask? Belt and suspenders — each layer covers the other's blind spots:

  • --dangerously-skip-permissions bypasses settings.json permission rules but not the hook, so the hook still prompts in YOLO sessions.
  • "Yes, don't ask again" bypasses the hook for that command but not the settings.json permission rule, so the rule still prompts on subsequent invocations.
  • The hook does full-command regex matching, not just prefix matching — it can distinguish --force push from plain push, catch plain git branch <name> branch creation, and match inside compound commands. It also emits tailored confirmation messages (e.g., "git force-push detected — confirm before proceeding").
  • The settings.json rule needs no extra code execution — it is declarative, evaluated by the harness directly, and easy to audit or extend per project.

Related Pull Requests

  • None.

Changes

  • .claude/settings.json — new permissions.ask entries for git push, git commit, git reset --hard, git branch -D, git checkout -b, git switch -c|-C, and gh pr create|merge|close.
  • .claude/hooks/check-dangerous-commands.py — new GIT_ASK_PATTERNS and check_git_for_ask(). Returns permissionDecision: ask with a tailored reason for the matched op.
  • .claude/hooks/check-secrets-file.py — small refactor so the block reason string can also be threaded into the debug log.
  • .claude/hooks/test-hooks.py — test harness now recognizes ASK as a third outcome alongside BLOCK / ALLOW; new GIT_ASK_TESTS table exercises positive and negative cases for every new pattern.
  • Both hook scripts gain an opt-in DEBUG flag that appends to .claude/hooks/hooks.log (gitignored, default off).
  • .gitignore — exclude .claude/hooks/*.log and Python bytecode.
  • CLAUDE.md — document git branch naming and PR conventions so future agent sessions follow them automatically.

Tasks

  • Manual testing @labkey-willm
  • Claude code review
  • Code review
  • TC and merge

labkey-martyp and others added 4 commits May 15, 2026 12:49
Add Git Branch Naming section covering develop, feature branches
(snake_case label), release-targeted branches, and SNAPSHOT/release
branches. Expand Pull Request Format to defer to pull_request_template.md
when present and require user confirmation before creating branches or PRs.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add .claude/hooks/*.log so debug-log output from the Claude Code
PreToolUse hooks isn't tracked. Add a top-level __pycache__/ and
*.pyc section so bytecode generated by those hooks (and any other
Python under .claude/) stays out of the repo.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds permissions.ask entries so that git push/commit/reset --hard,
branch -D/creation, and gh pr create/merge/close require user
confirmation before executing.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
check-dangerous-commands.py now emits permissionDecision=ask for
git/gh write operations (push, commit, reset --hard, branch -D,
branch creation, gh pr create/merge/close). Both hook scripts gain
an opt-in DEBUG log written to .claude/hooks/hooks.log. The test
harness is extended to recognize ASK alongside BLOCK/ALLOW.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@labkey-martyp labkey-martyp requested review from a team, labkey-jeckels and labkey-willm May 15, 2026 23:11
@labkey-martyp labkey-martyp changed the title Claude Code: prompt before git/gh write operations Claude Code: prompt before git/gh write operations and PR instructions May 15, 2026
Add a Commit and PR Body Formatting section directing every commit and PR body to use one physical line per paragraph or bullet, with blank lines between paragraphs. GitHub renders these as GFM with hard-line-break enabled, so mid-paragraph newlines become visible <br>s. The Commit Messages and Pull Request Format sections reference the new rule.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@labkey-jeckels
Copy link
Copy Markdown
Contributor

I'm far from an expert on these files and patterns to use, but automated review gave these findings to consider. Will's been doing more integration with GitHub actions so I'll defer to him for testing purposes.

Finding #1 - Medium — Missing test coverage for branch creation commands — .claude/hooks/test-hooks.py

Issue: GIT_ASK_TESTS has no positive ASK cases for git checkout -b, git switch -c, git switch -C, or git branch (the branch creation alternatives in the sixth pattern). All three settings.json entries for these (checkout -b,
switch -c, switch -C) are also untested end-to-end.

Why it matters: If a pattern regression silently broke these patterns, the test suite would pass without detecting it. This is the highest-impact pattern because branch naming is the one documented convention that could most easily
diverge.

Suggestion: Add ASK cases to GIT_ASK_TESTS:
("git checkout -b", "git checkout -b fb_myFeature", "ASK"),
("git switch -c", "git switch -c fb_myFeature", "ASK"),
("git switch -C", "git switch -C fb_myFeature", "ASK"),
("git branch creation", "git branch fb_myFeature", "ASK"),
And a negative case to confirm git branch --list stays ALLOW.


Finding #2 - Low — (?:-\S+\s+)* doesn't capture two-token git global flags — .claude/hooks/check-dangerous-commands.py

Issue: All patterns use (?:-\S+\s+)* to skip git global flags. This works for single-token flags like git -C alone, but fails for two-token flags (git -C /path commit, git --work-tree=/path push). For example, git -C /some/repo
commit -m "..." would not match the commit ASK pattern because /some/repo is not a dash-prefixed token, so the repetition stops at -C and commit doesn't follow directly.

Why it matters: The hook silently allows through any git command prefixed with a two-token global flag, providing no confirmation prompt. In practice Claude Code rarely generates git -C ..., but it could.

Suggestion: Extend the optional-flags fragment to also skip non-flag arguments after known two-arg global flags. Alternatively, replace (?:-\S+\s+)* with a more permissive (?:\S+\s+)* with a bounded repetition (e.g., {0,4}) to skip
any leading tokens before the subcommand.


Finding #3 - Low — git branch creation not covered by settings.json — .claude/settings.json

Issue: The hook catches git branch as an ASK case, but settings.json only has Bash(git branch -D:). There is no Bash(git branch:) or equivalent entry. The PR description says the two layers are mutually redundant, but this
specific case has only one layer.

Why it matters: If a user clicks "Yes, don't ask again" on the hook prompt for a git branch command, subsequent identical commands bypass both layers (hook prompt is bypassed, and there is no settings rule to fall back on).

Suggestion: Add "Bash(git branch:*)" to permissions.ask in settings.json, or accept the single-layer coverage as intentional (since git branch creation is less destructive than the other operations).

The PreToolUse ASK check previously returned the first matching pattern only, so a compound command like `git commit && git push` would prompt as a commit-only ask. Iterate all matches, drop spans contained inside a wider earlier match, and join the distinct reasons so the user sees every write op in one prompt. Adds two compound-command cases to the hook test harness.
CLAUDE.md tells the agent to self-check before invoking gh pr edit, but neither the settings.json permissions.ask list nor the PreToolUse hook patterns covered it, so PR body/title rewrites would proceed silently. Add a matching settings.json entry, a GIT_ASK_PATTERNS regex, and a positive case in the hook test harness so both confirmation layers fire.
Sign up for free to 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.

2 participants