Skip to content

refactor: handle prepare-commit-msg hook installation errors#249

Merged
appleboy merged 1 commit intoappleboy:mainfrom
doodleEsc:main
Aug 26, 2025
Merged

refactor: handle prepare-commit-msg hook installation errors#249
appleboy merged 1 commit intoappleboy:mainfrom
doodleEsc:main

Conversation

@doodleEsc
Copy link
Contributor

@doodleEsc doodleEsc commented Aug 20, 2025

  • Add specific error handling for prepare-commit-msg hook installation

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where installing a commit hook could fail if the target hook file didn’t exist. The installer now proceeds to create the hook when the file is missing, while still preventing overwrites of existing hooks and surfacing other errors as before. This improves reliability and reduces setup friction.

- Add specific error handling for `prepare-commit-msg` hook installation

Signed-off-by: doodleEsc <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Aug 20, 2025

Walkthrough

Adjusts InstallHook() in git/git.go to ignore os.IsNotExist errors when checking the hook target path, allowing hook creation if the target file is missing. Existing-file errors still stop execution; other unexpected errors are returned unchanged. No other logic or API surface changes.

Changes

Cohort / File(s) Summary of Changes
Git hook installation error handling
git/git.go
Modified InstallHook() control flow: if file.IsFile(target) returns os.IsNotExist, proceed to create the hook; if file exists, return existing-file error; other errors are returned as before.

Sequence Diagram(s)

sequenceDiagram
  participant C as Caller
  participant G as InstallHook()
  participant FS as FileSystem

  C->>G: InstallHook(target)
  G->>FS: IsFile(target)
  alt Error is os.IsNotExist
    note right of G: Target missing → continue
    G->>FS: Create hook file and set permissions
    G-->>C: Success/creation result
  else File exists
    note right of G: Existing hook
    G-->>C: Error "hook file ... exist"
  else Other error
    note right of G: Unexpected error
    G-->>C: Propagate error
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I thump my paw—no file in sight,
I’ll stitch a hook by lunar light.
If one exists, I’ll softly stop;
If not, I’ll hop and neatly drop.
A nibble, a commit, a tidy trick—
The carrot compiles; the hooks click! 🥕

Tip

🔌 Remote MCP (Model Context Protocol) integration is now available!

Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

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.

Summary of Changes

Hello @doodleEsc, 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 refactors the error handling logic for the prepare-commit-msg Git hook installation. The change ensures that the installation process correctly distinguishes between actual errors and cases where the hook file simply does not exist, preventing unnecessary failures and improving the robustness of the hook installation.

Highlights

  • Refined Error Handling for Git Hook Installation: Modified the InstallHook function in git/git.go to refine error handling when checking for the existence of the prepare-commit-msg hook file. Previously, any error from file.IsFile would halt the process; now, os.IsNotExist errors are specifically ignored, allowing the installation to proceed if the file simply doesn't exist.
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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

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 issue 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.

@doodleEsc
Copy link
Contributor Author

#248 pr for this issue

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 correctly adds error handling for the prepare-commit-msg hook installation, specifically to ignore os.ErrNotExist when checking if the hook file exists. My review includes a suggestion to refactor the new logic for improved readability and maintainability.

@codecov
Copy link

codecov bot commented Aug 20, 2025

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 6.79%. Comparing base (0125d86) to head (09aa75e).
⚠️ Report is 207 commits behind head on main.

Files with missing lines Patch % Lines
git/git.go 0.00% 3 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (0125d86) and HEAD (09aa75e). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (0125d86) HEAD (09aa75e)
1 0
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #249       +/-   ##
==========================================
- Coverage   21.96%   6.79%   -15.18%     
==========================================
  Files           8      33       +25     
  Lines         346    1870     +1524     
==========================================
+ Hits           76     127       +51     
- Misses        264    1734     +1470     
- Partials        6       9        +3     
Flag Coverage Δ
go-1.24 6.79% <0.00%> (?)
go-1.25 6.79% <0.00%> (?)
macos-latest 6.79% <0.00%> (?)
ubuntu-latest 6.79% <0.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
git/git.go (4)

205-207: Avoid TOCTOU: create the hook atomically with O_EXCL.

Between the existence check and WriteFile, another process could create the file. Use an atomic create to prevent clobbering and surface an accurate “already exists” error.

-// Write the hook file with executable permissions (0o755)
-return os.WriteFile(target, content, 0o755) //nolint:gosec
+// Write the hook file with executable permissions (0o755) atomically
+f, err := os.OpenFile(target, os.O_WRONLY|os.O_CREATE|os.O_EXCL, 0o755) //nolint:gosec
+if err != nil {
+	if os.IsExist(err) {
+		return fmt.Errorf("hook file already exists: %s", target)
+	}
+	return err
+}
+defer f.Close()
+if _, err := f.Write(content); err != nil {
+	return err
+}
+return nil

197-197: Nit: standardize and clarify error messages (include path).

Clearer user-facing errors help troubleshooting.

-		return errors.New("hook file prepare-commit-msg exist")
+		return fmt.Errorf("hook file already exists: %s", target)
-		return errors.New("hook file prepare-commit-msg is not exist")
+		return fmt.Errorf("hook file does not exist: %s", target)

Also applies to: 223-223


218-221: UninstallHook: treat os.IsNotExist consistently with InstallHook.

Mirror the improved install behavior so a missing hook produces a friendly “does not exist” message instead of propagating a raw error.

 	exists, err := file.IsFile(target)
 	if err != nil {
-		return err
+		if os.IsNotExist(err) {
+			return fmt.Errorf("hook file does not exist: %s", target)
+		}
+		return err
 	}

8-8: Prefer filepath over path for OS-native paths.

path is for slash-separated paths; filepath handles OS-specific separators. Low risk, improves portability on Windows.

-import (
+import (
 	"errors"
 	"fmt"
 	"os"
 	"os/exec"
-	"path"
+	"path/filepath"
 	"strconv"
 	"strings"
-	target := path.Join(strings.TrimSpace(string(hookPath)), HookPrepareCommitMessageTemplate)
+	target := filepath.Join(strings.TrimSpace(string(hookPath)), HookPrepareCommitMessageTemplate)
-	target := path.Join(strings.TrimSpace(string(hookPath)), HookPrepareCommitMessageTemplate)
+	target := filepath.Join(strings.TrimSpace(string(hookPath)), HookPrepareCommitMessageTemplate)

Also applies to: 191-191, 217-217

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled by default for public repositories
  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2673494 and 09aa75e.

📒 Files selected for processing (1)
  • git/git.go (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
git/git.go (2)
cmd/hook.go (1)
  • init (10-13)
git/hook.go (1)
  • init (29-33)
🔇 Additional comments (1)
git/git.go (1)

193-195: Good fix: ignore os.IsNotExist so a missing hook can be created.

This aligns behavior with intent: a nonexistent target should not abort installation, while other errors still surface.

@appleboy appleboy added the enhancement New feature or request label Aug 26, 2025
@appleboy appleboy merged commit 6215f0a into appleboy:main Aug 26, 2025
13 of 15 checks passed
@appleboy
Copy link
Owner

@doodleEsc Thanks.

@appleboy appleboy mentioned this pull request Aug 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants