Conversation
…(GIT-70) Adds a git hook that automatically injects AI-Agent trailers into commit messages when an AI-assisted session is detected (via $GIT_MEM_AGENT or $CLAUDE_CODE env vars). Install with `git mem init --hooks`, remove with `git mem init --uninstall-hooks`. Wraps existing hooks safely. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds CLI flags Changes
Sequence DiagramsequenceDiagram
participant User as Developer
participant CLI
participant Init as init Command
participant HookMod as prepare-commit-msg Module
participant GitFS as Git Repository
participant Commit as Commit Process
User->>CLI: git-mem init --hooks
CLI->>Init: run init (hooks=true)
Init->>HookMod: installHook(cwd)
HookMod->>GitFS: locate .git (handle worktree)
HookMod->>GitFS: read existing prepare-commit-msg hook
Note over HookMod: if existing non-git-mem hook:\nrename backup and write wrapper
HookMod->>GitFS: write git-mem prepare-commit-msg hook
HookMod-->>Init: return IHookInstallResult
Init->>CLI: log result
CLI-->>User: installation result
User->>Commit: git commit -m "msg"
Commit->>GitFS: trigger prepare-commit-msg
GitFS->>HookMod: execute hook script
HookMod->>HookMod: check GIT_MEM_AGENT / CLAUDE_CODE
Note over HookMod: if AI session and not merge/squash/amend\nand trailer missing -> use interpret-trailers
HookMod->>GitFS: update commit message with AI-Agent trailer
HookMod-->>GitFS: exit
GitFS-->>User: commit created (possibly modified)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/hooks/prepare-commit-msg.ts`:
- Around line 32-35: Update the misleading comment above the case
"$COMMIT_SOURCE" in prepare-commit-msg.ts: remove or reword "amend" so the
comment accurately reflects that the hook skips only merge and squash commit
sources (since Git does not pass "amend" as a COMMIT_SOURCE). Reference the
existing case "$COMMIT_SOURCE" in the file and adjust the comment to say
something like "Skip merge/squash commits (amend is not passed as a
COMMIT_SOURCE)" or simply "Skip merge and squash commits."
- Around line 114-117: The wrapper currently embeds the absolute backupPath into
HOOK_SCRIPT when building wrapperScript which breaks if the repo is moved;
change the injected command to call the backup hook relative to the hook's
location (e.g., use "$(dirname "$0")/prepare-commit-msg.user-backup") instead of
the absolute backupPath so the wrapperScript will always find
prepare-commit-msg.user-backup regardless of repo location; update the
replacement logic that constructs wrapperScript (references: wrapperScript,
HOOK_SCRIPT, backupPath, prepare-commit-msg.user-backup) to substitute the
relative dirname invocation.
- Around line 95-124: The installHook function can overwrite an existing backup
at backupPath when wrapping an existing hook; before calling
renameSync(hookPath, backupPath) check existsSync(backupPath) and handle that
case (e.g., avoid overwriting by choosing a new unique backup name, abort/warn
and skip wrapping, or fail the install) so the original user hook isn't lost;
ensure the chosen behavior updates the wrapped boolean and the wrapper creation
logic that references backupPath, and keep isGitMemHook and hookPath checks
intact.
- Fix comment: "merge/squash" not "merge/squash/amend" (git has no amend source) - Guard against overwriting existing backup in installHook - Use relative path for backup hook reference in wrapper script - Validate --limit flag as positive integer in trailers command Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
# Conflicts: # src/cli.ts
There was a problem hiding this comment.
Pull request overview
This PR implements a passive AI-Agent trailer injection system via a Git prepare-commit-msg hook. The feature enables automatic tracking of AI-assisted commits by detecting AI session context through environment variables ($GIT_MEM_AGENT or $CLAUDE_CODE) and injecting standardized AI-Agent: trailers into commit messages using Git's native git interpret-trailers command.
Changes:
- Adds core hook implementation with install/uninstall functions and shell script logic
- Integrates hook management into the CLI via
--hooksand--uninstall-hooksoptions - Extends the trailer type system with
AI-Agentkey - Provides comprehensive test coverage (12 tests) covering install, wrap, uninstall, and commit message modification scenarios
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/hooks/prepare-commit-msg.ts |
Core implementation of hook install/uninstall logic and shell script generation |
tests/unit/hooks/prepare-commit-msg.test.ts |
Comprehensive test suite covering installation, wrapping, uninstallation, and commit message modification |
src/commands/init.ts |
Integration of hook install/uninstall into the init command workflow |
src/cli.ts |
CLI option definitions for --hooks and --uninstall-hooks |
src/domain/entities/ITrailer.ts |
Addition of AGENT constant to AI_TRAILER_KEYS |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { describe, it, before, after } from 'node:test'; | ||
| import assert from 'node:assert/strict'; | ||
| import { execFileSync } from 'child_process'; | ||
| import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, rmSync, mkdirSync } from 'fs'; |
There was a problem hiding this comment.
The import mkdirSync is unused. Remove it from the imports to keep the code clean.
| import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, rmSync, mkdirSync } from 'fs'; | |
| import { mkdtempSync, writeFileSync, readFileSync, existsSync, chmodSync, rmSync } from 'fs'; |
| COMMIT_MSG_FILE="$1" | ||
| COMMIT_SOURCE="$2" | ||
|
|
||
| # Skip merge/squash/amend commits |
There was a problem hiding this comment.
The comment says "Skip merge/squash/amend commits" but the case statement only skips merge and squash. If amend commits should also be skipped, add it to the case statement. Otherwise, update the comment to match the actual behavior.
| # Skip merge/squash/amend commits | |
| # Skip merge/squash commits |
| export function installHook(cwd?: string): IHookInstallResult { | ||
| const gitDir = findGitDir(cwd || process.cwd()); | ||
| const hooksDir = join(gitDir, 'hooks'); | ||
| const hookPath = join(hooksDir, 'prepare-commit-msg'); | ||
| const backupPath = join(hooksDir, 'prepare-commit-msg.user-backup'); | ||
|
|
||
| // Already installed — idempotent | ||
| if (isGitMemHook(hookPath)) { | ||
| return { installed: false, wrapped: false, hookPath }; | ||
| } | ||
|
|
||
| let wrapped = false; | ||
|
|
||
| // Existing non-git-mem hook — wrap it | ||
| if (existsSync(hookPath)) { | ||
| renameSync(hookPath, backupPath); | ||
| wrapped = true; | ||
|
|
||
| // Create a wrapper that calls the user's hook first, then ours | ||
| const wrapperScript = HOOK_SCRIPT.replace( | ||
| '#!/bin/sh', | ||
| `#!/bin/sh\n# Wrapped existing hook — original saved as prepare-commit-msg.user-backup\nif [ -x "${backupPath}" ]; then\n "${backupPath}" "$@" || exit $?\nfi`, | ||
| ); | ||
| writeFileSync(hookPath, wrapperScript); | ||
| } else { | ||
| writeFileSync(hookPath, HOOK_SCRIPT); | ||
| } | ||
|
|
||
| chmodSync(hookPath, 0o755); | ||
| return { installed: true, wrapped, hookPath }; | ||
| } |
There was a problem hiding this comment.
The installHook function assumes the hooks directory exists, but this may not always be true (e.g., in bare repositories or corrupted git directories). Consider adding mkdirSync(hooksDir, { recursive: true }) before line 109 to ensure the directory exists before attempting to write files to it.
| // Create a wrapper that calls the user's hook first, then ours | ||
| const wrapperScript = HOOK_SCRIPT.replace( | ||
| '#!/bin/sh', | ||
| `#!/bin/sh\n# Wrapped existing hook — original saved as prepare-commit-msg.user-backup\nif [ -x "${backupPath}" ]; then\n "${backupPath}" "$@" || exit $?\nfi`, |
There was a problem hiding this comment.
The wrapper script uses an absolute path ${backupPath} in the shell script. This creates a hardcoded absolute path in the hook file, which will break if the repository is moved or cloned. Consider using a relative path like ./prepare-commit-msg.user-backup or using $(dirname "$0")/prepare-commit-msg.user-backup instead.
| `#!/bin/sh\n# Wrapped existing hook — original saved as prepare-commit-msg.user-backup\nif [ -x "${backupPath}" ]; then\n "${backupPath}" "$@" || exit $?\nfi`, | |
| `#!/bin/sh\n# Wrapped existing hook — original saved as prepare-commit-msg.user-backup\nBACKUP_HOOK="$(dirname "$0")/prepare-commit-msg.user-backup"\nif [ -x "$BACKUP_HOOK" ]; then\n "$BACKUP_HOOK" "$@" || exit $?\nfi`, |
Summary
src/hooks/prepare-commit-msg.tswithinstallHook()/uninstallHook()functions$GIT_MEM_AGENTor$CLAUDE_CODEenv varsgit interpret-trailers --in-placefor proper trailer formatting (< 100ms).user-backuprename, restores on uninstall--hooksand--uninstall-hooksCLI options togit mem initAI-AgenttoAI_TRAILER_KEYSCloses GIT-70
Test plan
installHookcreates hook with fingerprint in.git/hooks/uninstallHookremoves hook and restores backupGIT_MEM_AGENTis setCLAUDE_CODEis set🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
--hooksand--uninstall-hooksCLI options to the init command to manage Git hook installation.Tests