feat: add git hook integration with install and uninstall commands#38
Merged
Merged
Conversation
|
Someone is attempting to deploy a commit to the nirvik34's projects Team on Vercel. A member of the Team first needs to authorize it. |
Contributor
|
Caution Review failedFailed to post review comments Warning
|
| Layer / File(s) | Summary |
|---|---|
CLI generate-only flag and wiring bin/smartcommit.ts, src/index.ts |
Added --generate-only to the CLI and generateOnly?: boolean to CliOptions. When set, run() prints the generated commit message to stdout and returns before confirmation/dry-run/commit. |
Hook installation implementation src/hooks/install.ts |
Added installHook(force = false) which resolves the Git hooks dir, writes a prepare-commit-msg script containing a gitbun marker that runs gitbun --generate-only, prevents overwriting unmarked hooks unless --force, attempts to chmod the hook, and logs or exits on fatal errors. |
Hook uninstallation implementation src/hooks/uninstall.ts |
Added uninstallHook() which locates the prepare-commit-msg hook, verifies it contains the gitbun marker, aborts if the marker is missing, and deletes the hook when validated. |
CLI hook commands bin/smartcommit.ts |
Added hooks install (with --force) and hooks uninstall subcommands that call the new hook management functions; renamed program from smartcommit to gitbun. |
Sequence Diagram
sequenceDiagram
participant User
participant CLI as gitbun CLI
participant Installer as installHook()
participant Hooks as .git/hooks/prepare-commit-msg
participant Generator as "gitbun --generate-only"
User->>CLI: gitbun hooks install
CLI->>Installer: installHook(force)
Installer->>Hooks: write hook script with gitbun marker
Hooks-->>User: hook installed message
User->>User: git commit (later)
Hooks->>Generator: execute with COMMIT_EDITMSG path
Generator-->>Hooks: stdout generated commit message
Hooks->>Hooks: prepend/replace commit message file when non-empty
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
- nirvik34/gitbun#42: Modifies
run(options)control flow; may interact withgenerateOnlyearly-return behavior. - nirvik34/gitbun#14: Changes templating/formatting in commit generation which affects output used by
--generate-only. - nirvik34/gitbun#25: Alters commit message generation pipeline that
--generate-onlyprints before performing commit.
Suggested labels
type: feature, level:intermediate, quality:clean, gssoc:approved
Suggested reviewers
- nirvik34
Poem
🐰 A hook I baked with nimble paws,
It whispers messages without a pause,
--generate-onlyhums so bright,
Install, uninstall—done just right,
Gitbun hops in at commit-time's cause.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | The title accurately summarizes the main changes: adding git hook integration with install and uninstall commands, which matches the primary focus of the changeset. |
| Linked Issues check | ✅ Passed | The PR implements the core requirements from #4: optional git hook support with install/uninstall commands, prepare-commit-msg hook integration, staged changes analysis, and lightweight optional implementation that doesn't disrupt manual workflows. |
| Out of Scope Changes check | ✅ Passed | All changes are within scope and directly support the linked issue #4 objectives: CLI commands and options for hook management, hook installation/uninstallation logic, and generate-only output for hook usage. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
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 help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
src/hooks/install.ts (2)
46-61: ⚡ Quick winUse octal notation for file permissions.
Line 49 passes
"755"as a string tochmodSync. Node.jsfs.chmodSyncexpects a numeric mode, and while strings are auto-converted, the idiomatic form is octal:0o755.♻️ Proposed fix
try { - chmodSync(hookPath, "755"); + chmodSync(hookPath, 0o755); } catch {🤖 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/hooks/install.ts` around lines 46 - 61, The chmodSync call is using a string mode ("755"); change it to the numeric octal form so permissions are set idiomatically and correctly — replace the chmodSync(hookPath, "755") invocation with chmodSync(hookPath, 0o755) (keep the surrounding try/catch and references to hookPath and HOOK_SCRIPT intact).
5-5: ⚡ Quick winConsider extracting the marker to a shared constant.
HOOK_MARKERis duplicated in bothinstall.tsanduninstall.ts. Extracting it to a shared constants file (e.g.,src/hooks/constants.ts) would prevent the definitions from drifting over time.🤖 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/hooks/install.ts` at line 5, HOOK_MARKER is duplicated between install.ts and uninstall.ts; extract it into a single exported constant in a new shared module (e.g., create an exported const HOOK_MARKER in a new constants.ts) and update both files to import that constant instead of declaring it locally, ensuring both modules reference the same identifier to avoid drift.
🤖 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/hooks/install.ts`:
- Around line 7-19: HOOK_SCRIPT contains a Unicode em dash in the marker string
and blindly redirects gitbun output into the commit message file; replace the em
dash with an ASCII hyphen/minus in the HOOK_SCRIPT marker and change the
generation step to run gitbun --generate-only, check its exit status, and only
write its stdout to COMMIT_MSG_FILE on success (otherwise preserve
COMMIT_MSG_FILE and log or print the error); reference HOOK_SCRIPT,
COMMIT_MSG_FILE and the gitbun --generate-only invocation when making the
change.
- Around line 21-29: The installHook implementation currently builds hooksDir
using process.cwd() which breaks when run from a subdirectory; replace that
logic in installHook to determine the repo git directory by calling git
rev-parse --git-dir (via execFileSync with encoding 'utf8' and .trim()) and then
set hooksDir = join(gitDir, "hooks"); apply the identical change to the
uninstall logic (e.g., the uninstallHook in src/hooks/uninstall.ts) so both
installHook and uninstallHook compute hooksDir from git rev-parse --git-dir
instead of process.cwd().
In `@src/hooks/uninstall.ts`:
- Around line 7-14: uninstallHook currently resolves the .git/hooks path using
process.cwd() (hookPath in uninstallHook), which can mislocate the repository
root; instead run git rev-parse --git-dir (e.g., via child_process.execSync or
spawnSync) to get the repository git directory, normalize that path and build
the hooks path from it, falling back to the existing join(process.cwd(), ".git",
"hooks", "prepare-commit-msg") only if the git command fails; update
uninstallHook to mirror the same git-dir resolution approach used in install.ts
so the hookPath calculation is consistent and robust.
---
Nitpick comments:
In `@src/hooks/install.ts`:
- Around line 46-61: The chmodSync call is using a string mode ("755"); change
it to the numeric octal form so permissions are set idiomatically and correctly
— replace the chmodSync(hookPath, "755") invocation with chmodSync(hookPath,
0o755) (keep the surrounding try/catch and references to hookPath and
HOOK_SCRIPT intact).
- Line 5: HOOK_MARKER is duplicated between install.ts and uninstall.ts; extract
it into a single exported constant in a new shared module (e.g., create an
exported const HOOK_MARKER in a new constants.ts) and update both files to
import that constant instead of declaring it locally, ensuring both modules
reference the same identifier to avoid drift.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 120eb231-648c-4622-8dd0-fde09f193047
📒 Files selected for processing (4)
bin/smartcommit.tssrc/hooks/install.tssrc/hooks/uninstall.tssrc/index.ts
Owner
|
@Harikishanth Please resolve the merge conflict |
…ration # Conflicts: # bin/smartcommit.ts # src/index.ts
nirvik34
added
type: feature
|
🎉 This PR is included in version 1.14.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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
2 participants
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.
Description
Add
gitbun hooks installandgitbun hooks uninstallcommands to manage aprepare-commit-msggit hook that automatically pre-fills the commit editor with a gitbun-generated message.Fixes #4
Type of change
GSSoC '26 Contribution Details
level:beginner
level:intermediate
level:advanced
level:critical
I have been assigned to this issue by a maintainer.
How Has This Been Tested?
npm testpasses locallynpm run lintpasses without errorsnpm run devand verified the outputChecklist:
Additional Notes
What was added:
src/hooks/install.ts— writes aprepare-commit-msghook to.git/hooks/using bare Node.jsfs, marks it with a gitbun identifier for safe removalsrc/hooks/uninstall.ts— removes the hook only if it was installed by gitbun, refuses to delete hooks it didn't createsrc/index.ts— added--generate-onlyflag that outputs the generated message to stdout without committing (used internally by the hook)bin/smartcommit.ts— addedhooks install [--force]andhooks uninstallsubcommandsUsage:
gitbun hooks install # installs prepare-commit-msg hook
gitbun hooks install --force # overwrites an existing hook
gitbun hooks uninstall # removes the hook (only if installed by gitbun)
How it works:
Once installed, running
git commitautomatically triggers gitbun to generate a commit message and pre-fill the editor. The user can review and edit before confirming. Runninggit commit -m "..."or amending skips the hook entirely.Existing flow is completely unaffected — users who never run
gitbun hooks installexperience zero change in behaviour.Summary by CodeRabbit