Skip to content

refactor(content-guards): simplify enforce-issue-limits (391 → 200 lines)#125

Merged
JacobPEvans merged 6 commits intomainfrom
refactor/simplify-issue-limits
Mar 14, 2026
Merged

refactor(content-guards): simplify enforce-issue-limits (391 → 200 lines)#125
JacobPEvans merged 6 commits intomainfrom
refactor/simplify-issue-limits

Conversation

@JacobPEvans
Copy link
Owner

@JacobPEvans JacobPEvans commented Mar 14, 2026

Summary

  • Rewrote enforce-issue-limits.py from 391 to 200 lines by removing over-engineered two-tier rate limiting and consolidating duplicate functions
  • Removed external config file dependency (rate-limit-config.json), gh api user call, and trusted/default user tiers — replaced with simple hardcoded constants
  • Merged 14 functions into 7: _gh_json() wrapper, _get_counts(), _count_recent(), _check_duplicate(), _block(), main()
  • Unified rate limit to 10/24h for both issues and PRs (was 5 default / 10 trusted)
  • Kept --author @me identity-based filtering (unforgeable, unlike label-based approach in closed PR fix(content-guards): scope rate limit to ai-created label #122)
  • Updated bats tests: rate limit thresholds from 5 → 10, assertion text for new message format
  • Closed PR fix(content-guards): scope rate limit to ai-created label #122 (label-based filtering is insecure — labels are user-settable)

Test plan

  • All 9 unit tests pass (python3 content-guards/scripts/test_enforce_issue_limits.py)
  • All 17 bats integration tests pass (bats tests/content-guards/enforce-issue-limits/)
  • Verify hook works in live Claude Code session with gh issue create

🤖 Generated with Claude Code

Greptile Summary

This PR performs a well-executed refactor of the enforce-issue-limits.py hook, cutting it nearly in half (391 → 200 lines) by eliminating the two-tier rate limiting system, removing external config file dependencies, and consolidating 14 functions into 7. The core security model is preserved — --author @me identity-based filtering remains, hard limits (50 total / 25 AI-created) are unchanged, and the fail-open behavior is maintained throughout.

  • Replaced get_issue_counts() + get_pr_counts() with a generic _get_counts(resource) — clean DRY win
  • Merged check_duplicate_pr() + check_duplicate_issue() + extract_flag_value() + normalize_title() into a single _check_duplicate() function
  • Unified rate limit to 10/24h for all users (was 5 default / 10 trusted) — simpler, and removes the gh api user call + config file attack surface
  • Regex-based command matching (_CMD_RE) replaces manual string checks; slightly broader (now matches gh issue edit as a no-op)
  • Minor display regression: PR-related block messages now show lowercase "prs" instead of "PRs" due to using the raw regex capture group instead of the formatted label variable
  • Tests properly updated with new thresholds (5 → 10) and adjusted assertion text

Confidence Score: 4/5

  • This PR is safe to merge — the refactor preserves all security invariants and only has minor cosmetic issues in output messages.
  • Score of 4 reflects a clean, well-tested refactor with no logic bugs. The only findings are style-level: lowercase "prs" in user-facing block messages and a harmless no-op match on gh issue edit. All 17 bats integration tests and 9 unit tests pass per the PR description, and the test updates correctly reflect the new behavior.
  • No files require special attention — enforce-issue-limits.py has minor output formatting nits but no functional issues.

Important Files Changed

Filename Overview
content-guards/scripts/enforce-issue-limits.py Major refactor from 391 to 200 lines — consolidates duplicate functions into generics, removes two-tier rate limiting and external config. Logic is clean and correct; minor style issue with lowercase "prs" in user-facing messages, and regex now matches gh issue edit (harmless no-op).
tests/content-guards/enforce-issue-limits/enforce-issue-limits.bats Tests correctly updated — rate limit thresholds bumped from 5 to 10, assertion text adjusted for new message format ("prs" lowercase, "Duplicate Issue"). The shared mock approach (single GH_RESPONSE for all gh calls) works because the script's functions gracefully ignore unexpected/missing fields.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[stdin: JSON hook input] --> B{Parse JSON}
    B -- invalid --> Z0[exit 0: allow]
    B -- valid --> C{Regex match\ngh issue/pr create/edit?}
    C -- no match --> Z0
    C -- match --> D[Extract resource + action]
    D --> E{action == create?}
    E -- yes --> F[_check_duplicate:\ngh list --state open\ncompare title words]
    F --> G{Duplicate found?}
    G -- yes --> BLOCK1[exit 2: Duplicate detected]
    G -- no --> H[_get_counts:\ngh list --state open\ncount total + ai-created]
    H --> I{total >= 50 or\nai-created >= 25?}
    I -- yes --> BLOCK2[exit 2: Hard limit exceeded]
    I -- no --> J{Rate limit check?}
    E -- no --> J
    J -- "create, or pr+edit" --> K[_count_recent:\ngh list --author @me\ncount last 24h]
    K --> L{recent >= 10?}
    L -- yes --> BLOCK3[exit 2: Rate limit exceeded]
    L -- no --> Z0
    J -- "issue+edit" --> Z0
Loading
Prompt To Fix All With AI
This is a comment left during a code review.
Path: content-guards/scripts/enforce-issue-limits.py
Line: 174-176

Comment:
**User-facing messages display "prs" instead of "PRs"**

When `resource` is `"pr"`, these lines produce messages like `"Total prs: 10/50"` and `"AI-created prs: 0/25"`. The old code displayed `"Total PRs:"` which reads much more naturally. The properly-cased `label` variable (`"PR"`) is already available — consider using it here.

```suggestion
            reasons.append(f"Total {label}s: {total}/{total_limit} (limit reached)")
        if ai_created >= ai_limit:
            reasons.append(f"AI-created {label}s: {ai_created}/{ai_limit} (limit reached)")
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: content-guards/scripts/enforce-issue-limits.py
Line: 193

Comment:
**Rate limit message also says "prs" lowercase**

Same issue as above: `f"{recent} {resource}s created..."``"10 prs created in the past 24 hours"`. Using `label` here too would keep the output consistent and human-readable.

```suggestion
                f"{recent} {label}s created in the past 24 hours (limit: {RATE_LIMIT_24H}).\n\n"
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: content-guards/scripts/enforce-issue-limits.py
Line: 36

Comment:
**Regex now matches `gh issue edit` (previously excluded)**

The regex `(issue|pr)\s+(create|edit)` matches four combinations: `gh issue create`, `gh issue edit`, `gh pr create`, `gh pr edit`. The old code only handled three — `gh issue edit` was not intercepted. In the new code, `gh issue edit` enters `main()` but hits none of the actual checks (duplicate, hard limit, and rate limit all skip it) and falls through to exit 0. Functionally harmless, but it means unnecessary `gh` API calls from `_check_duplicate`/`_get_counts`/`_count_recent` could fire if the conditions in `main()` were ever broadened. Was this intentional?

If you want to preserve the old behavior (exclude `gh issue edit`), you could refine the regex or add an early exit. If it's deliberate future-proofing, maybe drop a comment.

How can I resolve this? If you propose a fix, please make it concise.

Last reviewed commit: 0510e5f

Greptile also left 3 inline comments on this PR.

…00 lines

Remove over-engineered two-tier rate limiting (trusted/default user config,
external config file, gh api user call) in favor of simple hardcoded limits.
Consolidate duplicate functions into generic resource-based equivalents:

- Merge get_issue_counts/get_pr_counts into _get_counts(resource)
- Merge check_duplicate_issue/check_duplicate_pr into _check_duplicate()
- Merge block_rate_limit/block_hard_limit/_block_duplicate into _block()
- Replace 6 separate subprocess blocks with single _gh_json() wrapper
- Use regex for command detection instead of string padding
- Unify rate limit to 10/24h for both issues and PRs (was 5 default/10 trusted)
- Remove unused os import and _DEFAULT_CONFIG/_load_rate_config/_get_current_user_id

Closes #122 (label-based filtering superseded - labels are user-settable).

(claude)
Copilot AI review requested due to automatic review settings March 14, 2026 17:58
@gemini-code-assist
Copy link

Summary of Changes

Hello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly refactors the enforce-issue-limits.py script, aiming to simplify its logic and improve maintainability. The changes remove a complex two-tier rate limiting system, external configuration, and consolidate multiple functions into a more cohesive set. The update standardizes rate limits for issues and pull requests, enhancing the clarity and robustness of the content guard mechanism while maintaining secure identity-based filtering.

Highlights

  • Code Simplification: Rewrote enforce-issue-limits.py, reducing its size from 391 to 200 lines by removing over-engineered two-tier rate limiting and consolidating duplicate functions.
  • Configuration Streamlining: Removed external configuration file dependency (rate-limit-config.json), the gh api user call, and the concept of trusted/default user tiers, replacing them with simple hardcoded constants.
  • Function Consolidation: Merged 14 functions into 7 core functions: _gh_json(), _get_counts(), _count_recent(), _check_duplicate(), _block(), and main().
  • Unified Rate Limits: Unified the 24-hour rolling rate limit to 10 for both issues and PRs, simplifying from the previous 5 (default) / 10 (trusted) scheme.
  • Identity-Based Filtering: Maintained --author @me for identity-based filtering, which is unforgeable and more secure than label-based approaches.
  • Test Updates: Updated bats tests to reflect the new rate limit thresholds (5 -> 10) and adjusted assertion text for the new message formats.
  • Security Improvement: Closed PR fix(content-guards): scope rate limit to ai-created label #122, which proposed label-based filtering, due to its inherent insecurity as labels are user-settable.
Changelog
  • content-guards/scripts/enforce-issue-limits.py
    • Simplified rate limiting logic by removing two-tier system and external configuration.
    • Consolidated multiple functions into a more streamlined set of helpers.
    • Updated hardcoded limits for issues and PRs.
    • Introduced a unified _block function for all blocking messages.
  • tests/content-guards/enforce-issue-limits/enforce-issue-limits.bats
    • Updated rate limit thresholds in tests from 5 to 10 to match new unified limits.
    • Adjusted assertion text for duplicate issue detection to reflect updated message format.
Activity
  • All 9 unit tests passed successfully.
  • All 17 bats integration tests passed successfully.
  • Verification of the hook in a live Claude Code session with gh issue create is pending.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request is a significant and well-executed refactoring of enforce-issue-limits.py. The simplification of the rate-limiting logic and removal of external dependencies greatly improves the script's clarity and maintainability. My review includes a couple of suggestions to further enhance the code structure by reducing duplication and consolidating related logic blocks, in line with the goals of this refactoring. Overall, this is an excellent improvement.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors the content-guards PreToolUse hook that rate-limits GitHub issue/PR creation by simplifying the implementation (removing tiered config logic) and updating integration tests to match the new unified limits and message formats.

Changes:

  • Simplified enforce-issue-limits.py by removing external config/tier logic and consolidating helpers into a smaller set of functions.
  • Unified 24h rate limits to a single constant (10/24h) for both issues and PRs, while keeping hard limits and duplicate-title blocking.
  • Updated bats integration tests to reflect the new 10/24h threshold and updated block message strings.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
content-guards/scripts/enforce-issue-limits.py Refactors the hook: unified limits, consolidated gh JSON wrapper, simplified duplicate/hard/rate-limit checks, and updated block messaging.
tests/content-guards/enforce-issue-limits/enforce-issue-limits.bats Updates integration tests for the new 10/24h rate limit and revised block-message text/casing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

The _CMD_RE regex matches `gh issue edit` but no checks apply to that
combination. Add explicit early return to avoid unnecessary processing.

(claude)
The _block() helper only indented the first line of multi-line details
strings. Now each non-empty line gets two-space indentation for
consistent formatting in stderr block messages.

(claude)
…miter

Consolidate two consecutive `if action == "create":` blocks into a single
block, grouping duplicate detection and hard limit checks together.

(claude)
…sing

Extract duplicated title normalization regex into _normalize_title() helper.
Remove unnecessary .lower() on label in block messages — "PRs" and "Issues"
read better than "prs" and "issues" in user-facing output.

(claude)
…t messages

TC5 and TC6 assertions updated to match "Issues"/"PRs" (from label variable)
instead of lowercase "issues"/"prs" (from resource variable).

(claude)
@JacobPEvans JacobPEvans merged commit 27c862a into main Mar 14, 2026
5 checks passed
@JacobPEvans JacobPEvans deleted the refactor/simplify-issue-limits branch March 14, 2026 18:27
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