Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 38 additions & 6 deletions src/application/handlers/CommitMsgHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,32 +48,39 @@ export class CommitMsgHandler implements IEventHandler<ICommitMsgEvent> {
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);
await this.appendTrailers(event.commitMsgPath, basicTrailers, event.cwd);
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', {
Expand All @@ -97,6 +104,31 @@ export class CommitMsgHandler implements IEventHandler<ICommitMsgEvent> {
}
}

/**
* 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.
Expand Down
11 changes: 10 additions & 1 deletion src/hooks/commit-msg.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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 -qiE "^Merge (branch|pull request|remote-tracking)" && exit 0

# Skip fixup/squash commits (will be squashed later)
head -1 "$COMMIT_MSG_FILE" | grep -qiE "^(fixup|squash|amend)! " && exit 0

# Skip revert commits (auto-generated)
head -1 "$COMMIT_MSG_FILE" | grep -qiE '^Revert "' && exit 0
Comment on lines +38 to +45
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

Optional: avoid reading the first line three times.

You can capture the first line once and reuse it for the three skip checks to reduce subprocesses and keep the script a bit cleaner.

♻️ Optional refactor
-# Skip merge commits (auto-generated by git)
-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 -qiE "^(fixup|squash|amend)! " && exit 0
-
-# Skip revert commits (auto-generated)
-head -1 "$COMMIT_MSG_FILE" | grep -qiE '^Revert "' && exit 0
+# Skip merge commits (auto-generated by git)
+FIRST_LINE=$(head -n 1 "$COMMIT_MSG_FILE")
+printf '%s' "$FIRST_LINE" | grep -qiE "^Merge (branch|pull request|remote-tracking)" && exit 0
+
+# Skip fixup/squash commits (will be squashed later)
+printf '%s' "$FIRST_LINE" | grep -qiE "^(fixup|squash|amend)! " && exit 0
+
+# Skip revert commits (auto-generated)
+printf '%s' "$FIRST_LINE" | grep -qiE '^Revert "' && exit 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Skip merge commits (auto-generated by git)
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 -qiE "^(fixup|squash|amend)! " && exit 0
# Skip revert commits (auto-generated)
head -1 "$COMMIT_MSG_FILE" | grep -qiE '^Revert "' && exit 0
# Skip merge commits (auto-generated by git)
FIRST_LINE=$(head -n 1 "$COMMIT_MSG_FILE")
printf '%s' "$FIRST_LINE" | grep -qiE "^Merge (branch|pull request|remote-tracking)" && exit 0
# Skip fixup/squash commits (will be squashed later)
printf '%s' "$FIRST_LINE" | grep -qiE "^(fixup|squash|amend)! " && exit 0
# Skip revert commits (auto-generated)
printf '%s' "$FIRST_LINE" | grep -qiE '^Revert "' && exit 0
🤖 Prompt for AI Agents
In `@src/hooks/commit-msg.ts` around lines 38 - 45, Read the commit message first
line once into a variable (e.g., first_line) and reuse it for the three checks
instead of calling head -1 "$COMMIT_MSG_FILE" each time; then update the three
guards that reference COMMIT_MSG_FILE (the grep checks for "^Merge (branch|pull
request|remote-tracking)", "^(fixup|squash|amend)! ", and '^Revert "') to test
against the saved variable (first_line) and keep the same exit 0 behavior when a
match is found.


# 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')
Expand Down
153 changes: 144 additions & 9 deletions tests/integration/hooks/hook-commit-msg.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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 (200 char limit in buildTrailers)
const decisionMatch = modifiedMsg.match(/AI-Decision: (.+)/);
if (decisionMatch) {
assert.ok(decisionMatch[1].length <= 200, 'Trailer content should be truncated to 200 chars');
}
Comment on lines +692 to +713
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Make the truncation test assert the decision trailer exists.

Right now the test passes even if AI-Decision is missing, so it won’t catch truncation regressions.

✅ Tighten the assertion
-      const decisionMatch = modifiedMsg.match(/AI-Decision: (.+)/);
-      if (decisionMatch) {
-        assert.ok(decisionMatch[1].length <= 200, 'Trailer content should be truncated to 200 chars');
-      }
+      const decisionMatch = modifiedMsg.match(/AI-Decision: (.+)/);
+      assert.ok(decisionMatch, 'Should include AI-Decision trailer');
+      assert.ok(decisionMatch[1].length <= 200, 'Trailer content should be truncated to 200 chars');
🤖 Prompt for AI Agents
In `@tests/integration/hooks/hook-commit-msg.test.ts` around lines 692 - 713, The
test "should handle very long commit message" currently only checks truncation
if the decision trailer exists; update the assertions to first assert that the
AI-Decision trailer is present (e.g., assert.ok(decisionMatch, 'AI-Decision
trailer must exist')) before asserting its length, locating the relevant
variables/expressions commitMsg, msgPath, runHook('commit-msg', ...),
modifiedMsg, and decisionMatch so that the test fails if AI-Decision is missing
and still enforces the 200-char truncation from buildTrailers.

});
});
});
6 changes: 3 additions & 3 deletions tests/unit/hooks/commit-msg.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'));
});

Expand Down Expand Up @@ -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 });
Expand Down Expand Up @@ -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 v4');
assert.ok(content.includes('git-mem hook commit-msg'), 'Should include git-mem command');

// Second install should be idempotent
Expand Down