fix: hook improvements and init defaults (GIT-85)#55
Merged
Conversation
Because users need to know about the new commit-msg hook feature that automatically adds AI trailers. AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101
Using the commit-msg hook because it automatically adds AI trailers. AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101
This reverts commit 3bf8016.
- Remove --hooks flag from init, hooks now install automatically - Change commit-msg skip check from AI-Agent to AI-Memory-Id - This allows prepare-commit-msg and commit-msg to work together - Bump commit-msg hook to v3 Because both hooks need to run: prepare-commit-msg adds basic trailers, commit-msg adds the full analysis (Decision/Gotcha/Convention, Tags, etc). AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Decision: both hooks need to run: prepare-commit-msg adds basic trailers, AI-Confidence: medium AI-Tags: application, handlers, commands, hooks, tests, integration, unit, pattern:because-clause AI-Lifecycle: project AI-Memory-Id: d8ed5636
Skip adding AI-Agent and AI-Model in commit-msg if prepare-commit-msg already added them. Both hooks now work together correctly. AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Gotcha: prevent duplicate Agent/Model trailers. Skip adding AI-Agent and AI-Model in commit-msg if prepare-commit-msg AI-Confidence: medium AI-Tags: application, handlers, typescript AI-Lifecycle: project AI-Memory-Id: 70ce8b50
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughDetects completed analysis via AI-Memory-Id (not AI-Agent), threads existing commit message into trailer builders to avoid re-adding Agent/Model trailers, makes hook installation unconditional in init and adds configureNotesPush, bumps hook fingerprint to v3, and updates related tests and changelog. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
git-mem init now configures remote.origin.push to include refs/notes/* so notes travel with commits on regular git push. AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Decision: auto-configure git to push notes with commits. git-mem init now configures remote.origin.push to include refs/notes/* AI-Confidence: medium AI-Tags: commands, typescript AI-Lifecycle: project AI-Memory-Id: ff173fa7
There was a problem hiding this comment.
Pull request overview
This PR updates git-mem’s init + hook behavior so hooks are installed by default, commit-msg no longer incorrectly short-circuits after prepare-commit-msg runs, and Agent/Model trailers aren’t duplicated when both hooks execute.
Changes:
- Make
git-mem initalways install prepare-commit-msg, commit-msg, and post-commit hooks; remove the--hooksCLI flag. - Update commit-msg hook skip logic to key off
AI-Memory-Id:and bump hook fingerprint to v3. - Prevent commit-msg from re-adding
AI-Agent/AI-Modelwhen already present (e.g., from prepare-commit-msg).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/hooks/commit-msg.test.ts | Updates expected commit-msg hook fingerprint to v3 in unit tests. |
| tests/integration/hooks/hook-commit-msg.test.ts | Adjusts integration skip-condition test to use AI-Memory-Id instead of AI-Agent. |
| src/hooks/commit-msg.ts | Bumps hook fingerprint to v3 and changes shell-level “skip if already analyzed” check to AI-Memory-Id. |
| src/commands/init.ts | Removes hooks option and installs hooks unconditionally during init; uninstall removes all hooks. |
| src/cli.ts | Removes --hooks flag from CLI; updates --uninstall-hooks description. |
| src/application/handlers/CommitMsgHandler.ts | Changes “already analyzed” detection to AI-Memory-Id and skips adding Agent/Model if already present. |
| CHANGELOG.md | Adds an Unreleased entry describing commit-msg “smart commit analysis”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/commands/init.ts`:
- Around line 79-109: The configureNotesPush function overwrites existing
remote.origin.push entries when there is a single refspec; instead preserve
existing refspecs by reading all current entries (the existing variable from
execFileSync), splitting/counting them, and then: if no entries exist use
execFileSync with 'remote.origin.push' (no --add) to set the heads refspec, if
one or more entries exist do NOT call the non---add setter (avoid the
execFileSync that writes '+refs/heads/*:refs/heads/*' without --add), and always
add the notes refspec using the '--add' form only when 'refs/notes' is missing;
update the code paths around the two execFileSync calls so the heads setter is
conditional on existing count and the notes setter uses '--add' to avoid
clobbering user refspecs.
In `@tests/unit/hooks/commit-msg.test.ts`:
- Around line 104-106: The failure message for the first assertion is stale
("Should be upgraded to v1") while the test actually checks for
'git-mem:commit-msg v3'; update the assertion message passed to assert.ok (the
second argument) to reflect v3 (e.g., "Should be upgraded to v3") so the error
message matches the checked value; locate the assertion using
readFileSync(hookPath, 'utf8') and the assert.ok call that checks for
'git-mem:commit-msg v3'.
- Fix assertion message: v1 → v3 in commit-msg.test.ts - Clarify AI-Memory-Id comment wording in hook and handler - Fix configureNotesPush to preserve existing user refspecs Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com> AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Gotcha: address PR review comments (GIT-73). - Fix assertion message: v1 → v3 in commit-msg.test.ts AI-Confidence: medium AI-Tags: application, handlers, commands, hooks, tests, unit AI-Lifecycle: project AI-Memory-Id: 605c296b
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/application/handlers/CommitMsgHandler.ts`:
- Around line 100-151: Both buildBasicTrailers and buildTrailers duplicate the
same Agent/Model presence check and resolution logic; extract a small helper
(e.g., addAgentAndModelTrailers or appendAgentModelIfMissing) that accepts the
existingMessage and the trailers array (or returns an array) and encapsulates
calls to this.agentResolver.resolveAgent(), this.agentResolver.resolveModel(),
and pushes { key: AI_TRAILER_KEYS.AGENT|MODEL, value } only when missing; then
replace the duplicated blocks in buildBasicTrailers and buildTrailers with calls
to that helper to keep logic DRY and avoid future divergence.
Comment on lines
100
to
151
| /** | ||
| * Build basic AI-Agent and AI-Model trailers only (when autoAnalyze is false). | ||
| * Skips if already present from prepare-commit-msg. | ||
| */ | ||
| private buildBasicTrailers(): ITrailer[] { | ||
| private buildBasicTrailers(existingMessage: string): ITrailer[] { | ||
| const trailers: ITrailer[] = []; | ||
| const agent = this.agentResolver.resolveAgent(); | ||
| const model = this.agentResolver.resolveModel(); | ||
| const hasAgent = existingMessage.includes('AI-Agent:'); | ||
| const hasModel = existingMessage.includes('AI-Model:'); | ||
|
|
||
| if (agent) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.AGENT, value: agent }); | ||
| if (!hasAgent) { | ||
| const agent = this.agentResolver.resolveAgent(); | ||
| if (agent) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.AGENT, value: agent }); | ||
| } | ||
| } | ||
| if (model) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.MODEL, value: model }); | ||
| if (!hasModel) { | ||
| const model = this.agentResolver.resolveModel(); | ||
| if (model) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.MODEL, value: model }); | ||
| } | ||
| } | ||
|
|
||
| return trailers; | ||
| } | ||
|
|
||
| /** | ||
| * Build all AI-* trailers from the analysis result. | ||
| * Skips Agent/Model if they already exist (from prepare-commit-msg). | ||
| */ | ||
| private buildTrailers( | ||
| analysis: ReturnType<ICommitAnalyzer['analyze']>, | ||
| config: ICommitMsgConfig | ||
| config: ICommitMsgConfig, | ||
| existingMessage: string | ||
| ): ITrailer[] { | ||
| const trailers: ITrailer[] = []; | ||
|
|
||
| // Always add Agent and Model via injected resolver | ||
| const agent = this.agentResolver.resolveAgent(); | ||
| const model = this.agentResolver.resolveModel(); | ||
| // Only add Agent and Model if not already present (prepare-commit-msg may have added them) | ||
| const hasAgent = existingMessage.includes('AI-Agent:'); | ||
| const hasModel = existingMessage.includes('AI-Model:'); | ||
|
|
||
| if (agent) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.AGENT, value: agent }); | ||
| if (!hasAgent) { | ||
| const agent = this.agentResolver.resolveAgent(); | ||
| if (agent) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.AGENT, value: agent }); | ||
| } | ||
| } | ||
| if (model) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.MODEL, value: model }); | ||
| if (!hasModel) { | ||
| const model = this.agentResolver.resolveModel(); | ||
| if (model) { | ||
| trailers.push({ key: AI_TRAILER_KEYS.MODEL, value: model }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Optional: dedupe Agent/Model trailer construction.
Both trailer builders reimplement the same Agent/Model logic; a small helper would prevent divergence.
♻️ Possible refactor
+ private buildAgentModelTrailers(existingMessage: string): ITrailer[] {
+ const trailers: ITrailer[] = [];
+ const hasAgent = existingMessage.includes('AI-Agent:');
+ const hasModel = existingMessage.includes('AI-Model:');
+
+ if (!hasAgent) {
+ const agent = this.agentResolver.resolveAgent();
+ if (agent) trailers.push({ key: AI_TRAILER_KEYS.AGENT, value: agent });
+ }
+ if (!hasModel) {
+ const model = this.agentResolver.resolveModel();
+ if (model) trailers.push({ key: AI_TRAILER_KEYS.MODEL, value: model });
+ }
+ return trailers;
+ }
+
private buildBasicTrailers(existingMessage: string): ITrailer[] {
- const trailers: ITrailer[] = [];
- const hasAgent = existingMessage.includes('AI-Agent:');
- const hasModel = existingMessage.includes('AI-Model:');
- ...
- return trailers;
+ return this.buildAgentModelTrailers(existingMessage);
}
private buildTrailers(..., existingMessage: string): ITrailer[] {
- const trailers: ITrailer[] = [];
- const hasAgent = existingMessage.includes('AI-Agent:');
- const hasModel = existingMessage.includes('AI-Model:');
- ...
- return trailers;
+ const trailers: ITrailer[] = [
+ ...this.buildAgentModelTrailers(existingMessage),
+ ];
+ ...
+ return trailers;
}🤖 Prompt for AI Agents
In `@src/application/handlers/CommitMsgHandler.ts` around lines 100 - 151, Both
buildBasicTrailers and buildTrailers duplicate the same Agent/Model presence
check and resolution logic; extract a small helper (e.g.,
addAgentAndModelTrailers or appendAgentModelIfMissing) that accepts the
existingMessage and the trailers array (or returns an array) and encapsulates
calls to this.agentResolver.resolveAgent(), this.agentResolver.resolveModel(),
and pushes { key: AI_TRAILER_KEYS.AGENT|MODEL, value } only when missing; then
replace the duplicated blocks in buildBasicTrailers and buildTrailers with calls
to that helper to keep logic DRY and avoid future divergence.
This was referenced Feb 14, 2026
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
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.
Summary
git-mem initnow installs all hooks automatically (no--hooksflag needed)Changes
--hooksflag from init - hooks are core functionalityAI-Agent:toAI-Memory-Id:Test Plan
git-mem init -yinstalls all 3 hooksCloses GIT-85
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Changes
Tests