From f5f66f9991a16d9c87cd712a43d2dc38023949c5 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sat, 14 Feb 2026 17:48:28 +0000 Subject: [PATCH 1/2] feat: handle edge cases in commit-msg hook (GIT-85) Skip analysis for special commit types: - Merge commits (Merge branch/pull request/remote-tracking) - Fixup/squash/amend! commits (will be squashed later) - Revert commits (auto-generated) Added shouldSkipAnalysis() to CommitMsgHandler with pattern detection. Updated shell hook to v4 with grep-based skip conditions. Also handles: - Unicode/emoji in commit messages - Very long commit messages (content truncated to 200 chars) - Empty/whitespace-only messages Co-Authored-By: Claude Opus 4.5 AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Decision: handle edge cases in commit-msg hook (GIT-85). Skip analysis for special commit types: AI-Confidence: medium AI-Tags: application, handlers, hooks, tests, integration, unit AI-Lifecycle: project AI-Memory-Id: 172b6832 --- src/application/handlers/CommitMsgHandler.ts | 44 ++++- src/hooks/commit-msg.ts | 11 +- .../integration/hooks/hook-commit-msg.test.ts | 153 ++++++++++++++++-- tests/unit/hooks/commit-msg.test.ts | 6 +- 4 files changed, 195 insertions(+), 19 deletions(-) diff --git a/src/application/handlers/CommitMsgHandler.ts b/src/application/handlers/CommitMsgHandler.ts index 3f6d020e..e68e0261 100644 --- a/src/application/handlers/CommitMsgHandler.ts +++ b/src/application/handlers/CommitMsgHandler.ts @@ -48,7 +48,14 @@ export class CommitMsgHandler implements IEventHandler { return { handler: 'CommitMsgHandler', success: true }; } - // 3. Check autoAnalyze config - if false, only add basic Agent/Model trailers + // 3. Skip special commit types that don't benefit from analysis + const skipReason = this.shouldSkipAnalysis(message); + if (skipReason) { + this.logger.debug(`Skipping analysis: ${skipReason}`); + return { handler: 'CommitMsgHandler', success: true }; + } + + // 4. Check autoAnalyze config - if false, only add basic Agent/Model trailers if (!config.autoAnalyze) { this.logger.debug('autoAnalyze is false, adding only Agent/Model trailers'); const basicTrailers = this.buildBasicTrailers(message); @@ -56,24 +63,24 @@ export class CommitMsgHandler implements IEventHandler { return { handler: 'CommitMsgHandler', success: true }; } - // 4. Get staged files (only if inferTags is enabled) + // 5. Get staged files (only if inferTags is enabled) const stagedFiles = config.inferTags ? this.gitClient.diffStagedNames(event.cwd) : []; - // 5. Analyze the commit message + // 6. Analyze the commit message const analysis = this.commitAnalyzer.analyze(message, stagedFiles); - // 6. Check requireType config - skip if no type detected and requireType is true + // 7. Check requireType config - skip if no type detected and requireType is true if (config.requireType && !analysis.type) { this.logger.debug('No memory type detected and requireType is true, skipping'); return { handler: 'CommitMsgHandler', success: true }; } - // 7. Build trailers (skip Agent/Model if already present from prepare-commit-msg) + // 8. Build trailers (skip Agent/Model if already present from prepare-commit-msg) const trailers = this.buildTrailers(analysis, config, message); - // 8. Append trailers to the commit message using git interpret-trailers + // 9. Append trailers to the commit message using git interpret-trailers await this.appendTrailers(event.commitMsgPath, trailers, event.cwd); this.logger.info('Commit message analyzed and trailers added', { @@ -97,6 +104,31 @@ export class CommitMsgHandler implements IEventHandler { } } + /** + * Check if analysis should be skipped for special commit types. + * Returns the skip reason, or null if analysis should proceed. + */ + private shouldSkipAnalysis(message: string): string | null { + const firstLine = message.split('\n')[0] || ''; + + // Skip merge commits (auto-generated by git) + if (/^Merge (branch|pull request|remote-tracking)/i.test(firstLine)) { + return 'merge commit'; + } + + // Skip fixup/squash/amend commits (will be squashed later) + if (/^(fixup|squash|amend)! /i.test(firstLine)) { + return 'fixup/squash commit'; + } + + // Skip revert commits (auto-generated) + if (/^Revert "/i.test(firstLine)) { + return 'revert commit'; + } + + return null; + } + /** * Build basic AI-Agent and AI-Model trailers only (when autoAnalyze is false). * Skips if already present from prepare-commit-msg. diff --git a/src/hooks/commit-msg.ts b/src/hooks/commit-msg.ts index 383f0bc5..3b989681 100644 --- a/src/hooks/commit-msg.ts +++ b/src/hooks/commit-msg.ts @@ -17,7 +17,7 @@ import { execFileSync } from 'child_process'; const HOOK_FINGERPRINT_PREFIX = '# git-mem:commit-msg'; /** Full fingerprint with version — used for upgrade detection. */ -const HOOK_FINGERPRINT = `${HOOK_FINGERPRINT_PREFIX} v3`; +const HOOK_FINGERPRINT = `${HOOK_FINGERPRINT_PREFIX} v4`; /** * The shell hook script. @@ -35,6 +35,15 @@ COMMIT_MSG_FILE="$1" # Skip if full analysis already done (AI-Memory-Id indicates commit-msg hook has run) grep -q "^AI-Memory-Id:" "$COMMIT_MSG_FILE" && exit 0 +# Skip merge commits (auto-generated by git) +head -1 "$COMMIT_MSG_FILE" | grep -qE "^Merge (branch|pull request|remote-tracking)" && exit 0 + +# Skip fixup/squash commits (will be squashed later) +head -1 "$COMMIT_MSG_FILE" | grep -qE "^(fixup|squash|amend)! " && exit 0 + +# Skip revert commits (auto-generated) +head -1 "$COMMIT_MSG_FILE" | grep -qE '^Revert "' && exit 0 + # Escape values for safe JSON inclusion (handles quotes, backslashes) COMMIT_MSG_FILE_ESC=$(printf '%s' "$COMMIT_MSG_FILE" | sed 's/\\\\/\\\\\\\\/g; s/"/\\\\"/g') CWD_ESC=$(pwd | sed 's/\\\\/\\\\\\\\/g; s/"/\\\\"/g') diff --git a/tests/integration/hooks/hook-commit-msg.test.ts b/tests/integration/hooks/hook-commit-msg.test.ts index 534e6326..6c7dad33 100644 --- a/tests/integration/hooks/hook-commit-msg.test.ts +++ b/tests/integration/hooks/hook-commit-msg.test.ts @@ -534,7 +534,7 @@ Because REST conventions make the API predictable.`; assert.ok(modifiedMsg.includes('AI-Decision:'), 'Should detect decision from "because"'); }); - it('should handle merge commit message', () => { + it('should skip merge commit message', () => { process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; const commitMsg = `Merge branch 'feature/auth' into main @@ -551,19 +551,17 @@ Because REST conventions make the API predictable.`; assert.equal(result.status, 0); - // Should add basic trailers even for merge commits + // Should NOT add trailers to merge commits const modifiedMsg = readFileSync(msgPath, 'utf8'); - assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should add trailers to merge commit'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip merge commits'); }); - it('should handle amend commit message', () => { + it('should skip merge pull request commit', () => { process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; - // Simulating an amend - the message file may have existing content from previous commit - const commitMsg = `feat: original feature + const commitMsg = `Merge pull request #123 from feature/auth -# This is an amended commit -Previously: fix typo`; +Add authentication feature`; const msgPath = createCommitMsgFile(repoDir, commitMsg); @@ -575,7 +573,144 @@ Previously: fix typo`; assert.equal(result.status, 0); const modifiedMsg = readFileSync(msgPath, 'utf8'); - assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should add trailers to amended commit'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip merge PR commits'); + }); + + it('should skip fixup! commits', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + const commitMsg = 'fixup! feat: original commit'; + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip fixup commits'); + }); + + it('should skip squash! commits', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + const commitMsg = 'squash! feat: original commit'; + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip squash commits'); + }); + + it('should skip amend! commits', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + const commitMsg = 'amend! feat: original commit'; + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip amend! commits'); + }); + + it('should skip revert commits', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + const commitMsg = `Revert "feat: add problematic feature" + +This reverts commit abc1234.`; + + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(!modifiedMsg.includes('AI-Agent:'), 'Should skip revert commits'); + }); + + it('should handle regular amend (git commit --amend)', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + // Regular amend - message doesn't start with "amend!" + // Just modifying a previous commit message + const commitMsg = `feat: original feature (amended) + +Updated the implementation details.`; + + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + // Regular amends should still get trailers + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should add trailers to regular amended commit'); + }); + + it('should handle unicode and emoji in commit message', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + const commitMsg = 'feat: add 日本語 support 🎉\n\nThis adds internationalization because users need 多言語 support.'; + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(modifiedMsg.includes('🎉'), 'Should preserve emoji'); + assert.ok(modifiedMsg.includes('日本語'), 'Should preserve unicode'); + assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should add trailers'); + }); + + it('should handle very long commit message', () => { + process.env.GIT_MEM_AGENT = 'TestAgent/2.0'; + + // Create a long commit message (> 1000 chars) + const longBody = 'This is a detailed explanation. '.repeat(50); + const commitMsg = `feat: add comprehensive feature\n\n${longBody}\n\nBecause this decision will help with scaling.`; + const msgPath = createCommitMsgFile(repoDir, commitMsg); + + const result = runHook('commit-msg', { + commit_msg_path: msgPath, + cwd: repoDir, + }); + + assert.equal(result.status, 0); + + const modifiedMsg = readFileSync(msgPath, 'utf8'); + assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should handle long messages'); + // Content should be truncated in trailers + const decisionMatch = modifiedMsg.match(/AI-Decision: (.+)/); + if (decisionMatch) { + assert.ok(decisionMatch[1].length <= 250, 'Trailer content should be truncated'); + } }); }); }); diff --git a/tests/unit/hooks/commit-msg.test.ts b/tests/unit/hooks/commit-msg.test.ts index a4430a8c..7bcfd647 100644 --- a/tests/unit/hooks/commit-msg.test.ts +++ b/tests/unit/hooks/commit-msg.test.ts @@ -39,7 +39,7 @@ describe('installCommitMsgHook', () => { const content = readFileSync(result.hookPath, 'utf8'); assert.ok(content.includes('#!/bin/sh')); - assert.ok(content.includes('git-mem:commit-msg v3')); + assert.ok(content.includes('git-mem:commit-msg v4')); assert.ok(content.includes('git-mem hook commit-msg')); }); @@ -74,7 +74,7 @@ describe('installCommitMsgHook', () => { // Installed hook should contain both fingerprint and wrapper reference const content = readFileSync(hookPath, 'utf8'); - assert.ok(content.includes('git-mem:commit-msg v3')); + assert.ok(content.includes('git-mem:commit-msg v4')); assert.ok(content.includes('user-backup')); } finally { rmSync(freshRepo, { recursive: true, force: true }); @@ -102,7 +102,7 @@ describe('installCommitMsgHook', () => { assert.equal(result.wrapped, false); const content = readFileSync(hookPath, 'utf8'); - assert.ok(content.includes('git-mem:commit-msg v3'), 'Should be upgraded to v3'); + assert.ok(content.includes('git-mem:commit-msg v4'), 'Should be upgraded to v3'); assert.ok(content.includes('git-mem hook commit-msg'), 'Should include git-mem command'); // Second install should be idempotent From ab122cd56a61814bf1bd4f0ae2b48953e3671709 Mon Sep 17 00:00:00 2001 From: Tony Casey Date: Sat, 14 Feb 2026 17:53:24 +0000 Subject: [PATCH 2/2] fix: address PR review comments (GIT-85) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add -i flag to grep for case-insensitive matching (matches TS handler) - Fix assertion message: v3 → v4 - Fix truncation test: 250 → 200 chars (matches code) Co-Authored-By: Claude Opus 4.5 AI-Agent: Claude-Code/2.1.42 AI-Model: claude-opus-4-5-20251101 AI-Gotcha: address PR review comments (GIT-85). - Add -i flag to grep for case-insensitive matching (matches TS handler) AI-Confidence: medium AI-Tags: hooks, tests, integration, unit AI-Lifecycle: project AI-Memory-Id: 9f67cb62 --- src/hooks/commit-msg.ts | 6 +++--- tests/integration/hooks/hook-commit-msg.test.ts | 4 ++-- tests/unit/hooks/commit-msg.test.ts | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/hooks/commit-msg.ts b/src/hooks/commit-msg.ts index 3b989681..6ca86942 100644 --- a/src/hooks/commit-msg.ts +++ b/src/hooks/commit-msg.ts @@ -36,13 +36,13 @@ COMMIT_MSG_FILE="$1" grep -q "^AI-Memory-Id:" "$COMMIT_MSG_FILE" && exit 0 # Skip merge commits (auto-generated by git) -head -1 "$COMMIT_MSG_FILE" | grep -qE "^Merge (branch|pull request|remote-tracking)" && exit 0 +head -1 "$COMMIT_MSG_FILE" | grep -qiE "^Merge (branch|pull request|remote-tracking)" && exit 0 # Skip fixup/squash commits (will be squashed later) -head -1 "$COMMIT_MSG_FILE" | grep -qE "^(fixup|squash|amend)! " && exit 0 +head -1 "$COMMIT_MSG_FILE" | grep -qiE "^(fixup|squash|amend)! " && exit 0 # Skip revert commits (auto-generated) -head -1 "$COMMIT_MSG_FILE" | grep -qE '^Revert "' && exit 0 +head -1 "$COMMIT_MSG_FILE" | grep -qiE '^Revert "' && exit 0 # Escape values for safe JSON inclusion (handles quotes, backslashes) COMMIT_MSG_FILE_ESC=$(printf '%s' "$COMMIT_MSG_FILE" | sed 's/\\\\/\\\\\\\\/g; s/"/\\\\"/g') diff --git a/tests/integration/hooks/hook-commit-msg.test.ts b/tests/integration/hooks/hook-commit-msg.test.ts index 6c7dad33..73c4b679 100644 --- a/tests/integration/hooks/hook-commit-msg.test.ts +++ b/tests/integration/hooks/hook-commit-msg.test.ts @@ -706,10 +706,10 @@ Updated the implementation details.`; const modifiedMsg = readFileSync(msgPath, 'utf8'); assert.ok(modifiedMsg.includes('AI-Agent:'), 'Should handle long messages'); - // Content should be truncated in trailers + // Content should be truncated in trailers (200 char limit in buildTrailers) const decisionMatch = modifiedMsg.match(/AI-Decision: (.+)/); if (decisionMatch) { - assert.ok(decisionMatch[1].length <= 250, 'Trailer content should be truncated'); + assert.ok(decisionMatch[1].length <= 200, 'Trailer content should be truncated to 200 chars'); } }); }); diff --git a/tests/unit/hooks/commit-msg.test.ts b/tests/unit/hooks/commit-msg.test.ts index 7bcfd647..b9bda7c1 100644 --- a/tests/unit/hooks/commit-msg.test.ts +++ b/tests/unit/hooks/commit-msg.test.ts @@ -102,7 +102,7 @@ describe('installCommitMsgHook', () => { assert.equal(result.wrapped, false); const content = readFileSync(hookPath, 'utf8'); - assert.ok(content.includes('git-mem:commit-msg v4'), 'Should be upgraded to v3'); + assert.ok(content.includes('git-mem:commit-msg v4'), 'Should be upgraded to v4'); assert.ok(content.includes('git-mem hook commit-msg'), 'Should include git-mem command'); // Second install should be idempotent