feat(hooks): add stale branch naming hooks#24
Conversation
Install post-checkout, pre-commit, and pre-push checks with configurable naming rules and safe bypasses. Closes #4
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds configurable branch naming rules, Git hook installation and checking, CLI commands, hook backup handling, tests, and README documentation. ChangesBranch Naming Hooks
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHook
participant hooksCheckCommand
participant getBranchNamingWarnings
participant logger
GitHook->>hooksCheckCommand: pass hook name and hook arguments
hooksCheckCommand->>getBranchNamingWarnings: validate current branch
getBranchNamingWarnings-->>hooksCheckCommand: return warnings
hooksCheckCommand->>logger: emit warnings or JSON result
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.md`:
- Around line 124-136: Add brief documentation immediately after the
branchNaming JSON example explaining requireTicket, requirePrefix,
ticketPattern, allowedPrefixes, and ignorePatterns. Use a concise table or
bullet list, describing each field’s purpose and expected value so users can
configure branch naming without consulting source code.
In `@src/core/config.ts`:
- Around line 60-66: Centralize the branch-naming defaults by defining and
exporting the shared default object from config.ts, then have
defaultConfig.branchNaming reuse it. Remove the duplicate DEFAULT_BRANCH_NAMING
values in hooks.ts and import the exported config default there, preserving the
existing requireTicket, requirePrefix, ticketPattern, allowedPrefixes, and
ignorePatterns values.
In `@src/core/hooks.ts`:
- Around line 52-55: Update globToRegex so the ? glob wildcard is escaped during
regex construction and then converted to . alongside the existing * to .*
conversion. Preserve literal regex-character escaping and all existing *
behavior.
- Around line 107-123: Update renderHookScript so the generated hook treats
git-broom invocation failures as non-blocking: detect when git-broom is
unavailable or its hooks check command fails, emit a warning, and exit
successfully with status 0. Preserve the existing backup-hook execution and
GIT_BROOM_FORCE command selection, while ensuring the hook still propagates
backup-hook failures.
In `@src/index.test.ts`:
- Around line 133-157: Add a focused test alongside “wires nested hook checks to
the repository and hook arguments” that invokes the nested hooks install CLI
command and asserts commandMocks.hooksInstallCommand receives the expected
parsed options and repository path, matching the existing hooks check wiring
coverage.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: d780882f-6d61-4d9e-9d5c-0d63de39445e
📒 Files selected for processing (10)
README.mdsrc/commands/hooks.tssrc/core/config.test.tssrc/core/config.tssrc/core/git.tssrc/core/hooks.test.tssrc/core/hooks.tssrc/index.test.tssrc/index.tssrc/types/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with Node.js >=24 and ESM-only modules.
Files:
src/index.test.tssrc/core/config.test.tssrc/core/hooks.test.tssrc/core/git.tssrc/core/config.tssrc/index.tssrc/commands/hooks.tssrc/types/index.tssrc/core/hooks.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use cross-platform path handling and avoid shell-specific assumptions because the repository must work on Windows.
Files:
src/index.test.tssrc/core/config.test.tssrc/core/hooks.test.tssrc/core/git.tssrc/core/config.tssrc/index.tssrc/commands/hooks.tssrc/types/index.tssrc/core/hooks.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.
Files:
src/index.test.tssrc/core/config.test.tssrc/core/hooks.test.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Favor small, composable helpers in
core/for logic that may need direct tests.
Files:
src/core/config.test.tssrc/core/hooks.test.tssrc/core/git.tssrc/core/config.tssrc/core/hooks.ts
src/core/config.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Maintain focused automated coverage for configuration resolution when changing configuration behavior.
Files:
src/core/config.test.ts
src/core/git.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/git.ts: Centralize Git command execution and repository inspection helpers insrc/core/git.ts; avoid scattering rawexeca('git', ...)calls.
Measure.gitsize using the established cross-platformdirSize()implementation; do not use Unix-only utilities such asdu.
Files:
src/core/git.ts
src/core/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/config.ts: Load.gitbroomrcby walking upward from the target repository path; parse it as JSON, ignore invalid files with a warning, and merge CLI overrides over file config over built-in defaults.
Keep configuration parsing tolerant enough that invalid user configuration does not break read-only inspection commands.
Files:
src/core/config.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
When changing command options, update the Commander wiring in
src/index.ts.
Files:
src/index.ts
src/types/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extend shared types in
src/types/index.tswhen command or core contracts change.
Files:
src/types/index.ts
🪛 ast-grep (0.44.1)
src/core/config.ts
[warning] 19-19: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 19-19: Do not use variable for regular expressions
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
src/core/hooks.ts
[warning] 81-81: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 53-53: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${escaped.replace(/\*/g, ".*")}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 81-81: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
🔇 Additional comments (10)
src/types/index.ts (1)
52-59: LGTM!Also applies to: 69-69
src/core/config.ts (1)
12-38: LGTM!Also applies to: 48-48
src/core/config.test.ts (1)
91-115: LGTM!src/core/git.ts (1)
3-3: LGTM!Also applies to: 20-24
src/core/hooks.ts (1)
36-50: LGTM!Also applies to: 65-105, 125-158
src/core/hooks.test.ts (1)
1-113: LGTM!src/index.ts (2)
27-29:getParentCommandtraversal is correct for nested commands.Walking to the topmost parent is necessary because
hooks checkis two levels deep (program → hooks → check). The old one-level traversal would returnhooksinstead ofprogram, breaking global option resolution. This change is backward compatible for existing one-level commands.
165-201: Hooks command wiring looks correct.The
installsubcommand correctly skips config resolution (not needed for installation), andcheckproperly resolves config with global overrides including JSON. The variadic[hookArgs...]argument and--hook/--forceoptions are correctly passed through tohooksCheckCommand.src/index.test.ts (1)
16-17: Mock setup for hooks commands is correct.The hoisted mocks and
vi.mockblock correctly wirehooksInstallCommandandhooksCheckCommandto the command mocks.Also applies to: 26-29
src/commands/hooks.ts (1)
36-44: 🩺 Stability & AvailabilityGIT_BROOM_FORCE bypass is already wired through
renderHookScriptpasses--forcewhenGIT_BROOM_FORCE=1, sohooksCheckCommandreceives the bypass viaoptions.force.> Likely an incorrect or invalid review comment.
Centralize branch naming defaults, support question-mark globs, and let unavailable or failing hook checks warn without blocking Git operations. Document the configuration fields and cover nested hook installation wiring.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 59 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/config.ts`:
- Line 48: Ensure resolved configurations do not share the mutable
DEFAULT_BRANCH_NAMING object: update the branchNaming default in configSchema
and the corresponding defaultConfig value to create an independent deep-cloned
instance for each resolution, using a factory or established cloning utility
while preserving the existing default contents.
- Around line 20-37: The current validation in branchNamingSchema only checks
whether ticketPattern compiles; no code change is required for the noted ReDoS
risk under the local-config threat model. If broader deployment requires defense
in depth, add a regex-complexity validation to ticketPattern before it is
consumed by the hooks logic, while preserving the existing compilation check and
downstream matching behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 2cbc356f-1218-4dcc-b958-c427c733b881
📒 Files selected for processing (10)
README.mdsrc/commands/hooks.tssrc/core/config.test.tssrc/core/config.tssrc/core/git.tssrc/core/hooks.test.tssrc/core/hooks.tssrc/index.test.tssrc/index.tssrc/types/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with Node.js >=24 and ESM-only modules.
Files:
src/core/config.test.tssrc/types/index.tssrc/core/git.tssrc/index.test.tssrc/core/hooks.tssrc/core/config.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Favor small, composable helpers in
core/for logic that may need direct tests.
Files:
src/core/config.test.tssrc/core/git.tssrc/core/hooks.tssrc/core/config.tssrc/core/hooks.test.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use cross-platform path handling and avoid shell-specific assumptions because the repository must work on Windows.
Files:
src/core/config.test.tssrc/types/index.tssrc/core/git.tssrc/index.test.tssrc/core/hooks.tssrc/core/config.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.ts
src/core/config.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Maintain focused automated coverage for configuration resolution when changing configuration behavior.
Files:
src/core/config.test.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.
Files:
src/core/config.test.tssrc/index.test.tssrc/core/hooks.test.ts
src/types/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extend shared types in
src/types/index.tswhen command or core contracts change.
Files:
src/types/index.ts
src/core/git.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/git.ts: Centralize Git command execution and repository inspection helpers insrc/core/git.ts; avoid scattering rawexeca('git', ...)calls.
Measure.gitsize using the established cross-platformdirSize()implementation; do not use Unix-only utilities such asdu.
Files:
src/core/git.ts
src/core/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/config.ts: Load.gitbroomrcby walking upward from the target repository path; parse it as JSON, ignore invalid files with a warning, and merge CLI overrides over file config over built-in defaults.
Keep configuration parsing tolerant enough that invalid user configuration does not break read-only inspection commands.
Files:
src/core/config.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
When changing command options, update the Commander wiring in
src/index.ts.
Files:
src/index.ts
🪛 ast-grep (0.44.1)
src/core/hooks.ts
[warning] 66-66: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
[warning] 38-38: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${escaped.replace(/\*/g, ".*").replace(/\\\?/g, ".")}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 66-66: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
src/core/config.ts
[warning] 27-27: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 27-27: Do not use variable for regular expressions
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (16)
src/types/index.ts (1)
52-58: LGTM!Also applies to: 69-69
src/core/config.ts (1)
5-13: LGTM! The centralization ofDEFAULT_BRANCH_NAMINGhere and the Zod schema validation (including the RegExp compile check and inner defaults) are well done. The past review comment about duplicating defaults betweenconfig.tsandhooks.tshas been addressed.Also applies to: 20-37
src/core/config.test.ts (1)
91-114: LGTM! Good focused test verifying that partialbranchNamingconfig correctly fills in defaults via the Zod schema.README.md (2)
13-13: LGTM! The documentation clearly explains the branch naming hooks feature, configuration fields, installation, and bypass options. The field table addresses the prior review feedback about documentingbranchNamingfields.Also applies to: 34-34, 124-153
152-152: 🎯 Functional CorrectnessNo change needed:
GIT_BROOM_FORCE=1is already handled in the installed hook wrapper.> Likely an incorrect or invalid review comment.src/core/git.ts (1)
3-3: LGTM!Also applies to: 20-24
src/core/hooks.ts (4)
37-40: LGTM! Past review concern about?glob wildcard handling has been addressed —?is now escaped in the character class and converted to.via the second replace.
50-76: LGTM!getBranchNamingReasonsandgetBranchNamingWarningscorrectly handle prefix/ticket validation, protected branch filtering, and ignore patterns.Also applies to: 78-90
92-113: LGTM! Past review concern about blocking git operations has been addressed — the script now captures thegit-broomexit status and exits 0 with a warning on failure, while still propagating backup hook failures.
115-144: LGTM!installHookscorrectly handles fresh installs, re-install detection viaHOOK_MARKER, and backup preservation.isHookNameis a clean type guard.Also applies to: 146-148
src/core/hooks.test.ts (1)
1-113: LGTM! Tests cover branch naming warning generation, valid branch acceptance, protected/ignored branch filtering, and hook installation with backup preservation.src/commands/hooks.ts (1)
1-60: LGTM!hooksInstallCommandis a clean wrapper, andhooksCheckCommandcorrectly handles hook normalization, branch checkout detection, force bypass, and JSON output.src/index.ts (2)
11-11: LGTM!getParentCommandcorrectly traverses to the topmost parent, enabling global option resolution for nested subcommands.Also applies to: 27-29
165-201: LGTM! Thehookscommand group withinstallandchecksubcommands is correctly wired —installpasses the repo path,checkpasses config with hook/force/hookArgs options.src/index.test.ts (2)
16-17: LGTM! Mock setup for both hook commands is consistent with the existing pattern.Also applies to: 26-29
133-166: LGTM! Past review concern about missinghooks installCLI wiring test has been addressed. Bothhooks checkandhooks installwiring are now covered.
Create fresh deep-cloned branch naming defaults for each config resolution and cover the isolation behavior with a regression test.
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 26 minutes. |
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/hooks.test.ts`:
- Around line 67-78: Add focused coverage in the existing “does not warn for
protected or ignored branches” test for the `?` wildcard handled by
`globToRegex`: configure `ignorePatterns` with `release/?` and assert that a
matching branch such as `release/1` produces no warnings, while preserving the
existing protected-branch and `*`-pattern assertions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3a44a449-be9c-40d3-a462-f235d3569d66
📒 Files selected for processing (10)
README.mdsrc/commands/hooks.tssrc/core/config.test.tssrc/core/config.tssrc/core/git.tssrc/core/hooks.test.tssrc/core/hooks.tssrc/index.test.tssrc/index.tssrc/types/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with Node.js >=24 and ESM-only modules.
Files:
src/core/config.test.tssrc/core/git.tssrc/index.test.tssrc/types/index.tssrc/core/config.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.tssrc/core/hooks.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Favor small, composable helpers in
core/for logic that may need direct tests.
Files:
src/core/config.test.tssrc/core/git.tssrc/core/config.tssrc/core/hooks.test.tssrc/core/hooks.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use cross-platform path handling and avoid shell-specific assumptions because the repository must work on Windows.
Files:
src/core/config.test.tssrc/core/git.tssrc/index.test.tssrc/types/index.tssrc/core/config.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.tssrc/core/hooks.ts
src/core/config.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Maintain focused automated coverage for configuration resolution when changing configuration behavior.
Files:
src/core/config.test.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.
Files:
src/core/config.test.tssrc/index.test.tssrc/core/hooks.test.ts
src/core/git.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/git.ts: Centralize Git command execution and repository inspection helpers insrc/core/git.ts; avoid scattering rawexeca('git', ...)calls.
Measure.gitsize using the established cross-platformdirSize()implementation; do not use Unix-only utilities such asdu.
Files:
src/core/git.ts
src/types/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extend shared types in
src/types/index.tswhen command or core contracts change.
Files:
src/types/index.ts
src/core/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/config.ts: Load.gitbroomrcby walking upward from the target repository path; parse it as JSON, ignore invalid files with a warning, and merge CLI overrides over file config over built-in defaults.
Keep configuration parsing tolerant enough that invalid user configuration does not break read-only inspection commands.
Files:
src/core/config.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
When changing command options, update the Commander wiring in
src/index.ts.
Files:
src/index.ts
🪛 ast-grep (0.44.1)
src/core/config.ts
[warning] 31-31: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 31-31: Do not use variable for regular expressions
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
src/core/hooks.ts
[warning] 38-38: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${escaped.replace(/\*/g, ".*").replace(/\\\?/g, ".")}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 66-66: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 66-66: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (13)
src/types/index.ts (1)
52-59: LGTM!Also applies to: 69-69
src/core/config.ts (1)
5-17: LGTM! Past review concerns about shared mutable state and ReDoS have been addressed viastructuredCloneand the local-config threat model.Also applies to: 24-41, 52-66, 106-106
src/core/config.test.ts (1)
61-77: LGTM!Also applies to: 108-132
src/core/git.ts (1)
3-3: LGTM!Also applies to: 20-24
src/core/hooks.ts (3)
37-40: LGTM! The?glob wildcard handling has been addressed —?is escaped in the first pass then converted to.in the second pass.
92-113: LGTM! The hook script now gracefully handlesgit-broomunavailability by printing a warning and exiting 0, aligning with the PR's "warn, not block" objective.
50-90: LGTM!Also applies to: 115-148
src/core/hooks.test.ts (1)
47-65: LGTM!Also applies to: 80-113
src/commands/hooks.ts (2)
12-22: LGTM!
24-60: 🎯 Functional CorrectnessNo change needed The generated hook wrappers already pass
--forcewhenGIT_BROOM_FORCE=1, sohooksCheckCommanddoes not need to read the env var directly.> Likely an incorrect or invalid review comment.src/index.ts (1)
27-29: LGTM!Also applies to: 165-201
src/index.test.ts (1)
16-29: LGTM!Also applies to: 134-166
README.md (1)
13-13: LGTM!Also applies to: 34-34, 124-153
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/commands/hooks.ts`:
- Around line 24-59: Add focused tests for hooksCheckCommand covering
post-checkout file mode ("0"), branch mode ("1"), force bypass, warning
emission, and JSON output. Mock getCurrentBranch and logger interactions, and
assert each decision’s returned HookCheckResult plus the expected warning or
JSON behavior; retain existing forwarding coverage in src/index.test.ts.
In `@src/core/hooks.ts`:
- Around line 37-39: Replace the regex-based implementation of globToRegex with
a linear-time glob matcher for ignorePatterns, avoiding conversion of wildcard
stars into .* and RegExp compilation. Preserve the existing glob semantics for *
and ? and the anchored full-pattern match behavior, using the already validated
ticketPattern input.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 4a70f059-eb0b-47d9-87ef-d0986b248716
📒 Files selected for processing (10)
README.mdsrc/commands/hooks.tssrc/core/config.test.tssrc/core/config.tssrc/core/git.tssrc/core/hooks.test.tssrc/core/hooks.tssrc/index.test.tssrc/index.tssrc/types/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with Node.js >=24 and ESM-only modules.
Files:
src/core/git.tssrc/core/config.test.tssrc/types/index.tssrc/core/config.tssrc/index.test.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.tssrc/core/hooks.ts
src/core/git.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/git.ts: Centralize Git command execution and repository inspection helpers insrc/core/git.ts; avoid scattering rawexeca('git', ...)calls.
Measure.gitsize using the established cross-platformdirSize()implementation; do not use Unix-only utilities such asdu.
Files:
src/core/git.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Favor small, composable helpers in
core/for logic that may need direct tests.
Files:
src/core/git.tssrc/core/config.test.tssrc/core/config.tssrc/core/hooks.test.tssrc/core/hooks.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use cross-platform path handling and avoid shell-specific assumptions because the repository must work on Windows.
Files:
src/core/git.tssrc/core/config.test.tssrc/types/index.tssrc/core/config.tssrc/index.test.tssrc/core/hooks.test.tssrc/commands/hooks.tssrc/index.tssrc/core/hooks.ts
src/core/config.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Maintain focused automated coverage for configuration resolution when changing configuration behavior.
Files:
src/core/config.test.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.
Files:
src/core/config.test.tssrc/index.test.tssrc/core/hooks.test.ts
src/types/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extend shared types in
src/types/index.tswhen command or core contracts change.
Files:
src/types/index.ts
src/core/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/config.ts: Load.gitbroomrcby walking upward from the target repository path; parse it as JSON, ignore invalid files with a warning, and merge CLI overrides over file config over built-in defaults.
Keep configuration parsing tolerant enough that invalid user configuration does not break read-only inspection commands.
Files:
src/core/config.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
When changing command options, update the Commander wiring in
src/index.ts.
Files:
src/index.ts
🪛 ast-grep (0.44.1)
src/core/config.ts
[warning] 31-31: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 31-31: Do not use variable for regular expressions
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
src/core/hooks.ts
[warning] 38-38: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(^${escaped.replace(/\*/g, ".*").replace(/\\\?/g, ".")}$)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 66-66: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 66-66: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (17)
src/types/index.ts (1)
52-59: LGTM!Also applies to: 69-69
src/core/config.ts (1)
5-42: LGTM!Also applies to: 52-66, 106-106
src/core/config.test.ts (1)
61-77: LGTM!Also applies to: 108-132
README.md (2)
13-13: LGTM!Also applies to: 34-34
124-153:ignorePatternsalready supports*and?.matchesPatternusesglobToRegex, which expands*to.*and?to., so the README matches the implementation.> Likely an incorrect or invalid review comment.src/core/hooks.test.ts (3)
67-87: Assert that?rejects multiple characters.
release/1covers only the positive case. Also assert thatrelease/12is not ignored, so a regression from.to.*is detected.As per coding guidelines, “Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.”
Source: Coding guidelines
1-65: LGTM!
89-121: LGTM!src/core/git.ts (1)
3-3: LGTM!Also applies to: 20-23
src/core/hooks.ts (5)
1-35: LGTM!
42-48: LGTM!Also applies to: 50-63, 75-90
92-113: LGTM!
119-148: LGTM!
115-117: 🎯 Functional CorrectnessNo hook-path change needed.
getGitPath("hooks")already followscore.hooksPath(relative and absolute), soinstallHooksis writing to the effective hook directory.> Likely an incorrect or invalid review comment.src/commands/hooks.ts (1)
12-22: LGTM!src/index.ts (1)
11-11: LGTM!Also applies to: 27-29, 165-200
src/index.test.ts (1)
16-29: LGTM!Also applies to: 134-166
| export async function hooksCheckCommand( | ||
| config: BroomConfig, | ||
| options: { hook: string; force?: boolean; hookArgs?: string[] }, | ||
| cwd?: string, | ||
| ): Promise<HookCheckResult> { | ||
| const hook: HookName = isHookName(options.hook) ? options.hook : "pre-commit"; | ||
| const branch = await getCurrentBranch(cwd); | ||
| const isBranchCheckout = | ||
| hook !== "post-checkout" || | ||
| !options.hookArgs || | ||
| options.hookArgs.length === 0 || | ||
| options.hookArgs.at(-1) === "1"; | ||
| const warnings = | ||
| options.force || !isBranchCheckout | ||
| ? [] | ||
| : getBranchNamingWarnings(branch, config); | ||
| const result: HookCheckResult = { | ||
| hook, | ||
| branch, | ||
| warnings, | ||
| bypassed: Boolean(options.force), | ||
| }; | ||
|
|
||
| if (config.json) { | ||
| logger.json(result); | ||
| return result; | ||
| } | ||
|
|
||
| for (const warning of warnings) { | ||
| logger.warn( | ||
| `Branch "${warning.branch}" may become stale: ${warning.reasons.join("; ")}. ` + | ||
| "Use --force or Git --no-verify to bypass this warning.", | ||
| ); | ||
| } | ||
|
|
||
| return result; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Add direct coverage for hook-check decisions.
src/index.test.ts verifies forwarding only; it does not exercise post-checkout file mode (0), branch mode (1), force bypass, or emitted warning/JSON behavior. Add focused handler tests with mocked branch lookup and logger.
As per coding guidelines, “Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/commands/hooks.ts` around lines 24 - 59, Add focused tests for
hooksCheckCommand covering post-checkout file mode ("0"), branch mode ("1"),
force bypass, warning emission, and JSON output. Mock getCurrentBranch and
logger interactions, and assert each decision’s returned HookCheckResult plus
the expected warning or JSON behavior; retain existing forwarding coverage in
src/index.test.ts.
Source: Coding guidelines
Add focused hooks check coverage for checkout modes, bypassing, warnings, and JSON output.
|
@coderabbitai full review |
✅ Action performedFull review finished. |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/core/config.test.ts`:
- Around line 108-132: Add a test alongside “parses configurable branch naming
rules” that writes a non-compilable ticketPattern to .gitbroomrc, calls
resolveConfig, and asserts the configuration falls back to the default
branch-naming settings rather than retaining the invalid value.
In `@src/core/git.ts`:
- Around line 20-24: Add direct unit coverage for getGitPath, mocking git to
return both an absolute path and a relative path. Verify absolute results are
returned unchanged and relative results are resolved against the supplied cwd,
covering both branches of the normalization logic.
In `@src/core/hooks.ts`:
- Around line 63-89: The ticket validation in getBranchNamingReasons must not
execute arbitrary ticketPattern regexes that can catastrophically backtrack.
Replace the new RegExp(...).test(branch) path with a linear-time matcher, or
validate and restrict ticketPattern to a safe supported pattern shape before
matching, while preserving the existing missing-ticket reason and
invalid-pattern handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: a6d21957-8962-45d8-aade-01dc349dced7
📒 Files selected for processing (11)
README.mdsrc/commands/hooks.test.tssrc/commands/hooks.tssrc/core/config.test.tssrc/core/config.tssrc/core/git.tssrc/core/hooks.test.tssrc/core/hooks.tssrc/index.test.tssrc/index.tssrc/types/index.ts
📜 Review details
🧰 Additional context used
📓 Path-based instructions (9)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript with Node.js >=24 and ESM-only modules.
Files:
src/index.test.tssrc/core/git.tssrc/commands/hooks.test.tssrc/core/config.test.tssrc/index.tssrc/core/config.tssrc/commands/hooks.tssrc/types/index.tssrc/core/hooks.test.tssrc/core/hooks.ts
src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use cross-platform path handling and avoid shell-specific assumptions because the repository must work on Windows.
Files:
src/index.test.tssrc/core/git.tssrc/commands/hooks.test.tssrc/core/config.test.tssrc/index.tssrc/core/config.tssrc/commands/hooks.tssrc/types/index.tssrc/core/hooks.test.tssrc/core/hooks.ts
src/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Add focused automated coverage for configuration resolution, safety filters, and command behavior when touching those areas.
Files:
src/index.test.tssrc/commands/hooks.test.tssrc/core/config.test.tssrc/core/hooks.test.ts
src/core/git.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/git.ts: Centralize Git command execution and repository inspection helpers insrc/core/git.ts; avoid scattering rawexeca('git', ...)calls.
Measure.gitsize using the established cross-platformdirSize()implementation; do not use Unix-only utilities such asdu.
Files:
src/core/git.ts
src/core/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Favor small, composable helpers in
core/for logic that may need direct tests.
Files:
src/core/git.tssrc/core/config.test.tssrc/core/config.tssrc/core/hooks.test.tssrc/core/hooks.ts
src/core/config.test.ts
📄 CodeRabbit inference engine (AGENTS.md)
Maintain focused automated coverage for configuration resolution when changing configuration behavior.
Files:
src/core/config.test.ts
src/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
When changing command options, update the Commander wiring in
src/index.ts.
Files:
src/index.ts
src/core/config.ts
📄 CodeRabbit inference engine (AGENTS.md)
src/core/config.ts: Load.gitbroomrcby walking upward from the target repository path; parse it as JSON, ignore invalid files with a warning, and merge CLI overrides over file config over built-in defaults.
Keep configuration parsing tolerant enough that invalid user configuration does not break read-only inspection commands.
Files:
src/core/config.ts
src/types/index.ts
📄 CodeRabbit inference engine (AGENTS.md)
Extend shared types in
src/types/index.tswhen command or core contracts change.
Files:
src/types/index.ts
🪛 ast-grep (0.44.1)
src/core/config.ts
[warning] 31-31: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 31-31: Do not use variable for regular expressions
Context: new RegExp(value)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
src/core/hooks.ts
[warning] 79-79: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 79-79: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🔇 Additional comments (13)
src/types/index.ts (1)
52-59: LGTM!Also applies to: 69-69
src/core/config.ts (1)
5-42: LGTM!Also applies to: 52-66, 106-106
src/core/config.test.ts (1)
61-77: LGTM!src/commands/hooks.test.ts (1)
1-135: LGTM!README.md (2)
13-13: LGTM!Also applies to: 34-34, 124-153
152-152: No issue with theGIT_BROOM_FORCE=1bypass.src/core/git.ts (1)
3-3: LGTM!src/core/hooks.test.ts (1)
1-122: LGTM!src/core/hooks.ts (2)
1-61: LGTM!
91-161: LGTM!src/commands/hooks.ts (1)
1-60: LGTM!src/index.ts (1)
11-11: LGTM!Also applies to: 27-29, 165-201
src/index.test.ts (1)
16-17: LGTM!Also applies to: 26-29, 133-157, 159-166
| export async function getGitPath(path: string, cwd?: string): Promise<string> { | ||
| const gitPath = await git(['rev-parse', '--git-path', path], cwd) | ||
| return isAbsolute(gitPath) ? gitPath : resolve(cwd ?? process.cwd(), gitPath) | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
fd -i 'git.test'
rg -n -C2 'getGitPath' --type=ts -g '*.test.ts' -g '*.spec.ts'Repository: 404-PF/git-broom
Length of output: 154
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate all references to getGitPath and git-path usage.
rg -n -C 2 --hidden --glob '!**/node_modules/**' 'getGitPath|--git-path|rev-parse' .
# List test files that might cover git helpers.
fd -t f -i 'test' .
# Inspect the git helper source with line numbers.
if [ -f src/core/git.ts ]; then
wc -l src/core/git.ts
cat -n src/core/git.ts | sed -n '1,220p'
fiRepository: 404-PF/git-broom
Length of output: 8872
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show hooks tests around the mocked getGitPath usage.
if [ -f src/core/hooks.test.ts ]; then
wc -l src/core/hooks.test.ts
cat -n src/core/hooks.test.ts | sed -n '1,220p'
fi
# Search for any git-specific test files or direct assertions on normalization behavior.
fd -t f 'git' src
rg -n -C 2 'resolve\(cwd ?? process\.cwd\(\),|isAbsolute\(|getGitPath\(' src --glob '*.test.ts' --glob '*.spec.ts'Repository: 404-PF/git-broom
Length of output: 4481
Add a direct unit test for getGitPath normalization
The absolute/relative branching is only covered indirectly through src/core/hooks.test.ts mocks. A small test for absolute passthrough and relative resolution against cwd would lock in the cross-platform path handling here.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/git.ts` around lines 20 - 24, Add direct unit coverage for
getGitPath, mocking git to return both an absolute path and a relative path.
Verify absolute results are returned unchanged and relative results are resolved
against the supplied cwd, covering both branches of the normalization logic.
| function getBranchNamingReasons( | ||
| branch: string, | ||
| rules: BranchNamingConfig, | ||
| ): string[] { | ||
| const reasons: string[] = []; | ||
|
|
||
| if (rules.requirePrefix) { | ||
| const prefix = branch.split("/")[0]; | ||
| if (!rules.allowedPrefixes.includes(prefix ?? "")) { | ||
| reasons.push( | ||
| `missing a recognized prefix (${rules.allowedPrefixes.join(", ")})`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| if (rules.requireTicket) { | ||
| try { | ||
| if (!new RegExp(rules.ticketPattern).test(branch)) { | ||
| reasons.push(`missing a ticket number matching ${rules.ticketPattern}`); | ||
| } | ||
| } catch { | ||
| reasons.push("uses an invalid ticket pattern in .gitbroomrc"); | ||
| } | ||
| } | ||
|
|
||
| return reasons; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -n -C5 'ticketPattern' src/core/config.ts src/types/index.tsRepository: 404-PF/git-broom
Length of output: 2400
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== src/core/hooks.ts ==\n'
wc -l src/core/hooks.ts
sed -n '1,180p' src/core/hooks.ts
printf '\n== src/core/config.ts ==\n'
wc -l src/core/config.ts
sed -n '1,120p' src/core/config.ts
printf '\n== search for renderHookScript / ignorePatterns matcher ==\n'
rg -n -C3 'renderHookScript|ignorePatterns|ticketPattern|new RegExp' src/core src/typesRepository: 404-PF/git-broom
Length of output: 14939
🏁 Script executed:
#!/bin/bash
set -euo pipefail
node - <<'JS'
const { performance } = require('node:perf_hooks');
const regex = new RegExp('^(a+)+$');
for (const n of [10, 12, 14, 16, 18, 20, 22, 24]) {
const s = 'a'.repeat(n) + '!';
const t0 = performance.now();
regex.test(s);
const dt = performance.now() - t0;
console.log(`${n}\t${dt.toFixed(3)}ms`);
}
JSRepository: 404-PF/git-broom
Length of output: 245
Avoid evaluating ticketPattern as a raw regex
src/core/hooks.ts:78-83 still runs new RegExp(rules.ticketPattern).test(branch) on every hook invocation. .gitbroomrc only checks that the pattern compiles, so a pathological expression can still trigger catastrophic backtracking and stall commits/pushes. Use a linear matcher or constrain the allowed pattern shape.
🧰 Tools
🪛 ast-grep (0.44.1)
[warning] 79-79: Regular expression constructed from variable input detected. This can lead to Regular Expression Denial of Service (ReDoS) attacks if the variable contains malicious patterns. Use libraries like 'recheck' to validate regex safety or use static patterns.
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity
(regexp-from-variable)
[warning] 79-79: Do not use variable for regular expressions
Context: new RegExp(rules.ticketPattern)
Note: [CWE-1333] Inefficient Regular Expression Complexity. Security best practice.
(regexp-non-literal-typescript)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/hooks.ts` around lines 63 - 89, The ticket validation in
getBranchNamingReasons must not execute arbitrary ticketPattern regexes that can
catastrophically backtrack. Replace the new RegExp(...).test(branch) path with a
linear-time matcher, or validate and restrict ticketPattern to a safe supported
pattern shape before matching, while preserving the existing missing-ticket
reason and invalid-pattern handling.
|
@coderabbitai full review |
✅ Action performedFull review finished. Your plan includes PR reviews subject to rate limits. More reviews will be available in 46 minutes. |
…k names - Remove redundant nullish coalescing in prefix check - Clarify increment logic in glob matching algorithm - Add warning when unknown hook name is provided - Document shell choice for portability
writeFileSync with mode: 0o755 already sets permissions correctly. The subsequent chmodSync call was redundant.
- Cache ticketPattern RegExp per branch check to avoid redundant compilation - Clarify that /bin/sh usage is a Git constraint, not a portability choice - Document hook name interpolation as safe via enum literal - Add trailing newline to hooks.test.ts
- Add hook name to warning messages for better debugging context - Fix regex escaping in isProtectedBranch to handle special characters - Validate ignorePatterns for unmatched brackets to catch glob typos These improvements make the hooks feature more reliable and easier to debug when issues arise.
Split the monolithic 'does not warn for protected or ignored branches' test into three focused assertions: protected branches, * glob, and ? single-character glob with a negative case. Add a note to README explaining that installed hooks use #!/bin/sh and require Git Bash or WSL on Windows.
Add validation to detect ambiguous unbounded quantifiers in ticket patterns to prevent catastrophic backtracking. Patterns like `a{1,}a{1,}!` are now rejected, falling back to the default safe pattern.
1 participant
Summary
Adds Git hook integration that warns when branch names are likely to become stale.
Changes
git-broom hook install(also available ashooks)post-checkout,pre-commit, andpre-pushchecks.git-broom-backupfiles--force,GIT_BROOM_FORCE=1, and Git's--no-verifybypassTesting
npm run typechecknpm test -- --run(37 tests)npm run buildNote:
npm run lintis currently unavailable because the repository has no ESLint flat configuration.Related
Closes #4
Summary by CodeRabbit
git-broom hooks installandgit-broom hooks checkto install and validate supported Git hooks.branchNamingexamples, bypass behavior (--no-verify, forced checks), and hook backup/preservation details.