Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
📝 WalkthroughWalkthroughSwitches init to YAML-based config stored under a config dir (via Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 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)
No actionable comments were generated in the recent review. 🎉 Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/commands/init.ts`:
- Around line 250-261: The YAML parsing around existingGitMemConfig can return
null or a non-object causing deepMergeGitMemConfig to fail; update the logic
that builds existingGitMemConfig (the parseYaml call and its surrounding IIFE
used when existsSync(configPath) is true) to coerce the parseYaml result into a
safe object (e.g., treat null or non-object as {}), or wrap the entire
read+parse step in a try/catch that returns {} on any error, before calling
deepMergeGitMemConfig with buildGitMemConfig(); ensure you reference parseYaml,
existingGitMemConfig, deepMergeGitMemConfig and buildGitMemConfig when making
the change.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@src/commands/init.ts`:
- Line 312: The ensureGitignoreEntries call in init (ensureGitignoreEntries(cwd,
['.env', '.git-mem/'])) is missing '.git-mem.json'; update the argument list to
include '.git-mem.json' so it becomes ensureGitignoreEntries(cwd, ['.env',
'.git-mem/', '.git-mem.json']) to ensure the user-editable hooks config is
ignored.
|
|
||
| // ── .gitignore ───────────────────────────────────────────────── | ||
| ensureGitignoreEntries(cwd, ['.env', '.git-mem.json']); | ||
| ensureGitignoreEntries(cwd, ['.env', '.git-mem/']); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for .git-mem.json references in the codebase
rg -n "\.git-mem\.json" src/ --type ts
# Search for migration-related code and comments
rg -n "migrat|backward|compat" src/ --type ts -i
# Check the config loading mechanism mentioned in learnings
cat -n src/hooks/utils/config.tsRepository: TonyCasey/git-mem
Length of output: 4911
🏁 Script executed:
# Search for documentation about config changes
find . -maxdepth 2 -type f \( -name "README.md" -o -name "CHANGELOG.md" -o -name "CHANGES.md" \) -exec cat -n {} +
# Also check the init.ts file to see full context around line 312
cat -n src/commands/init.ts | sed -n '300,330p'Repository: TonyCasey/git-mem
Length of output: 6917
🏁 Script executed:
# Check for any test files that test migration scenarios
fd -e test.ts -e spec.ts | xargs rg -l "migration|git-mem\.json|backward" 2>/dev/null || echo "No migration tests found"
# Check the full init.ts to understand the broader context
wc -l src/commands/init.tsRepository: TonyCasey/git-mem
Length of output: 222
🏁 Script executed:
# Check if init.ts calls init-hooks.ts and understand the config architecture
sed -n '1,50p' src/commands/init.ts
sed -n '250,320p' src/commands/init.ts
# Check what init-hooks.ts is for (hook configuration vs general config)
head -30 src/commands/init-hooks.ts
# Check the IHookConfig interface to understand purpose
cat -n src/domain/interfaces/IHookConfig.tsRepository: TonyCasey/git-mem
Length of output: 8058
🏁 Script executed:
# Search for any migration logic in init.ts specifically
rg -n "migration|migrate|\.git-mem\.json|backward" src/commands/init.ts -C 3
# Check if there's any logic that references both .git-mem.json and .git-mem.yaml
rg -n "\.git-mem\.yaml|CONFIG_FILE|CONFIG_DIR" src/commands/init.ts -C 2Repository: TonyCasey/git-mem
Length of output: 485
🏁 Script executed:
# Confirm: does init-hooks.ts create .git-mem.json and is it being ignored?
grep -n "\.git-mem\.json" src/commands/init-hooks.ts | head -20
# Check what files are typically ignored in similar projects (user config files)
# Look at the comment/context around line 312
sed -n '305,320p' src/commands/init.ts
# Verify: .git-mem.json contains what kind of data
grep -A 5 "buildGitMemConfig\|buildHooksConfig" src/commands/init-hooks.ts | head -20Repository: TonyCasey/git-mem
Length of output: 1692
🏁 Script executed:
# Search for any gitignore related to .git-mem.json
rg -n "gitignore.*git-mem" src/ --type ts -i -C 2Repository: TonyCasey/git-mem
Length of output: 897
Add .git-mem.json to the gitignore entries list.
The .git-mem.json file is created by init-hooks and contains user-customizable hook configuration. It should be ignored alongside .git-mem/ and .env. Currently, line 312 only ignores '.env' and '.git-mem/', but .git-mem.json will be committed without being explicitly ignored, which breaks the expected pattern for user configuration files.
Update the ensureGitignoreEntries call to:
ensureGitignoreEntries(cwd, ['.env', '.git-mem/', '.git-mem.json']);
(Note: The two configuration files—.git-mem.json for hooks and .git-mem/.git-mem.yaml for general config—coexist and serve different purposes; there is no migration required between them.)
🤖 Prompt for AI Agents
In `@src/commands/init.ts` at line 312, The ensureGitignoreEntries call in init
(ensureGitignoreEntries(cwd, ['.env', '.git-mem/'])) is missing '.git-mem.json';
update the argument list to include '.git-mem.json' so it becomes
ensureGitignoreEntries(cwd, ['.env', '.git-mem/', '.git-mem.json']) to ensure
the user-editable hooks config is ignored.
Owner
Author
There was a problem hiding this comment.
This is a false positive. The purpose of this PR series is to migrate from .git-mem.json to .git-mem/.git-mem.yaml. After this migration:
.git-mem.jsonis no longer created by any code path- All config (both hooks and general) now lives in
.git-mem/.git-mem.yaml - The
.git-mem/directory entry in gitignore covers the new config file
The only remaining reference to .git-mem.json is a comment in IHookConfig.ts which should be updated as part of cleanup, but it doesn't affect functionality.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for any code that writes to .git-mem.json
rg -n "writeFileSync.*\.git-mem\.json|\.git-mem\.json.*writeFileSync" src/ --type ts
# Check references to .git-mem.json in the codebase
rg -n "\.git-mem\.json" src/ --type ts -C 2
# Check the config.ts file to see what it loads
cat src/hooks/utils/config.ts
# Check IHookConfig.ts for the comment mentioned
cat src/domain/interfaces/IHookConfig.ts
Length of output: 8718
@TonyCasey, thank you for the clarification! You're absolutely correct—I misunderstood the migration context.
Looking at the code:
src/hooks/utils/config.tsnow loads from.git-mem/.git-mem.yaml- The
initcommand (file under review) writes to.git-mem/.git-mem.yaml - The
.git-mem/directory entry in gitignore properly covers the new config file
My review comment was indeed a false positive. I apologize for the confusion.
As you mentioned, there are still references to .git-mem.json in comments throughout init-hooks.ts, IHookConfig.ts, and HookConfigLoader.ts that could be updated as part of cleanup. The init-hooks.ts file also still has code (lines 220, 275-276) that writes to .git-mem.json—is that command path being deprecated in favor of the unified init command?
In the git-mem project, configuration migrated from .git-mem.json to .git-mem/.git-mem.yaml (YAML format). The unified init command creates .git-mem/.git-mem.yaml and hooks read from it via src/hooks/utils/config.ts. The .git-mem/ gitignore entry covers the new YAML config file. The old .git-mem.json is no longer created by current code paths.
🧬 Code Graph Analysis Results
- File: src/hooks/utils/config.ts, lines 25-28
export function getConfigPath(cwd?: string): string {
const dir = cwd ?? process.cwd();
return join(dir, CONFIG_DIR, CONFIG_FILE);
}
- File: src/hooks/utils/config.ts, lines 35-38
export function getConfigDir(cwd?: string): string {
const dir = cwd ?? process.cwd();
return join(dir, CONFIG_DIR);
}
🧠 Learnings used
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to .git-mem.json : Use .git-mem.json at project root for hook configuration, which is loaded by src/hooks/utils/config.ts and must never throw
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 21
File: src/commands/init-hooks.ts:69-75
Timestamp: 2026-02-12T15:35:31.520Z
Learning: In src/commands/**/*.ts: Commands that require domain services (IMemoryService, IContextService, ILiberateService, etc.) should use the Awilix DI container pattern via createContainer() and resolve services from container.cradle. Simple configuration/initialization commands that only perform file I/O operations (e.g., init-mcp, init-hooks) should accept an optional ILogger parameter directly without DI container overhead.
</learning>
<!-- [remove_learning]
e6547207-68e3-47f5-b1b7-4eedeeeec6ea
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to tests/integration/**/*.test.ts : Write integration tests that create real temporary git repos in os.tmpdir(), run actual git commands, and clean up in after() hooks in tests/integration/
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 49
File: src/hooks/post-commit.ts:25-34
Timestamp: 2026-02-14T10:03:02.254Z
Learning: In src/hooks/post-commit.ts, the post-commit hook script intentionally suppresses stderr with `2>/dev/null` to keep commits fast and non-intrusive; users can debug by running `git-mem hook post-commit` manually with `GIT_MEM_LOG_LEVEL=debug`.
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 21
File: src/application/services/ContextFormatter.ts:8-9
Timestamp: 2026-02-12T15:35:08.022Z
Learning: In the git-mem project (CommonJS with esModuleInterop: true), use `.js` extensions ONLY when importing from external ESM packages like `modelcontextprotocol/sdk` (e.g., `from 'modelcontextprotocol/sdk/server/mcp.js'`). Do NOT use `.js` extensions for internal relative imports (e.g., `from '../../infrastructure/di'` is correct).
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 22
File: src/hooks/utils/config.ts:30-50
Timestamp: 2026-02-12T17:16:42.308Z
Learning: In src/hooks/utils/config.ts, the .git-mem.json config loader intentionally avoids per-field type validation because the config is internal to hook entry points and already falls back to defaults on parse failure; adding type guards for individual fields would add complexity without meaningful safety improvement.
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to src/infrastructure/**/*.ts : Use ASCII separators \x1e (record) and \x1f (field) for git log parsing instead of null bytes
Learnt from: TonyCasey
Repo: TonyCasey/lisa PR: 211
File: src/lib/skills/shared/services/MemoryService.ts:393-418
Timestamp: 2026-02-06T17:09:35.417Z
Learning: In src/lib/skills/shared/services/MemoryService.ts consolidate method with git-mem backend, returning a placeholder retainedUuid ('new-merged-fact') for the merge action is acceptable because git-mem's remember() API returns Promise<boolean> (success/failure) rather than the created fact's ID. The delete + remember pattern is the standard way to update memories in git-mem.
Owner
Author
There was a problem hiding this comment.
The init-hooks command is being updated in PR #62 (git-91 branch) which is stacked on top of this PR. After that PR, init-hooks.ts will also use .git-mem/.git-mem.yaml. The init-hooks command remains as a standalone option for users who only want Claude Code hook integration without the full init setup.
There was a problem hiding this comment.
@TonyCasey, thank you for the clarification! That makes perfect sense—I understand the migration strategy now:
- PR
#61(current): Migrates the unifiedinitcommand to use.git-mem/.git-mem.yaml - PR
#62(git-91, stacked): Will migrateinit-hookscommand to also use.git-mem/.git-mem.yaml - Both commands will coexist, with
init-hooksserving users who only want Claude Code hook integration
The stacked PR approach is clean and keeps each change focused. I appreciate you explaining the roadmap!
✏️ Learnings added
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 61
File: src/commands/init.ts:312-312
Timestamp: 2026-02-14T20:47:23.329Z
Learning: In the git-mem project, hook and general configuration is stored in `.git-mem/.git-mem.yaml` (YAML format) within the `.git-mem/` directory. The unified `init` command and `init-hooks` command both create/update `.git-mem/.git-mem.yaml`, which is loaded by `src/hooks/utils/config.ts`. The `.git-mem/` directory is gitignored to keep user configuration out of version control.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
⛔ Learnings removed
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to .git-mem.json : Use .git-mem.json at project root for hook configuration, which is loaded by src/hooks/utils/config.ts and must never throw
🧠 Learnings used
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to .git-mem.json : Use .git-mem.json at project root for hook configuration, which is loaded by src/hooks/utils/config.ts and must never throw
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 21
File: src/commands/init-hooks.ts:69-75
Timestamp: 2026-02-12T15:35:31.520Z
Learning: In src/commands/**/*.ts: Commands that require domain services (IMemoryService, IContextService, ILiberateService, etc.) should use the Awilix DI container pattern via createContainer() and resolve services from container.cradle. Simple configuration/initialization commands that only perform file I/O operations (e.g., init-mcp, init-hooks) should accept an optional ILogger parameter directly without DI container overhead.
</learning>
<!-- [remove_learning]
e6547207-68e3-47f5-b1b7-4eedeeeec6ea
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to tests/integration/**/*.test.ts : Write integration tests that create real temporary git repos in os.tmpdir(), run actual git commands, and clean up in after() hooks in tests/integration/
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 49
File: src/hooks/post-commit.ts:25-34
Timestamp: 2026-02-14T10:03:02.254Z
Learning: In src/hooks/post-commit.ts, the post-commit hook script intentionally suppresses stderr with `2>/dev/null` to keep commits fast and non-intrusive; users can debug by running `git-mem hook post-commit` manually with `GIT_MEM_LOG_LEVEL=debug`.
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 21
File: src/application/services/ContextFormatter.ts:8-9
Timestamp: 2026-02-12T15:35:08.022Z
Learning: In the git-mem project (CommonJS with esModuleInterop: true), use `.js` extensions ONLY when importing from external ESM packages like `modelcontextprotocol/sdk` (e.g., `from 'modelcontextprotocol/sdk/server/mcp.js'`). Do NOT use `.js` extensions for internal relative imports (e.g., `from '../../infrastructure/di'` is correct).
Learnt from: TonyCasey
Repo: TonyCasey/git-mem PR: 22
File: src/hooks/utils/config.ts:30-50
Timestamp: 2026-02-12T17:16:42.308Z
Learning: In src/hooks/utils/config.ts, the .git-mem.json config loader intentionally avoids per-field type validation because the config is internal to hook entry points and already falls back to defaults on parse failure; adding type guards for individual fields would add complexity without meaningful safety improvement.
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to src/infrastructure/**/*.ts : Use ASCII separators \x1e (record) and \x1f (field) for git log parsing instead of null bytes
Learnt from: TonyCasey
Repo: TonyCasey/lisa PR: 211
File: src/lib/skills/shared/services/MemoryService.ts:393-418
Timestamp: 2026-02-06T17:09:35.417Z
Learning: In src/lib/skills/shared/services/MemoryService.ts consolidate method with git-mem backend, returning a placeholder retainedUuid ('new-merged-fact') for the merge action is acceptable because git-mem's remember() API returns Promise<boolean> (success/failure) rather than the created fact's ID. The delete + remember pattern is the standard way to update memories in git-mem.
Learnt from: CR
Repo: TonyCasey/lisa PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-06T11:57:30.539Z
Learning: Applies to src/project/.claude/hooks/**/*.ts : Hooks must be written in TypeScript in src/project/.claude/hooks/ and compiled to .claude/hooks/ via npm run build
Learnt from: CR
Repo: TonyCasey/git-mem PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-02-14T09:43:48.659Z
Learning: Applies to src/commands/hook.ts : Set hard 10s timeout on hooks via setupShutdown() to prevent Claude Code from hanging
Learnt from: CR
Repo: TonyCasey/lisa PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-01-30T20:19:52.042Z
Learning: Applies to {.claude/hooks/**/*.{ts,js},.opencode/plugin/**/*.{ts,js},src/project/.claude/hooks/**/*.{ts,tsx}} : Hook handlers in `.claude/hooks/` and `.opencode/plugin/` must be auto-generated from canonical handlers in `src/lib/application/handlers/hooks/`. Do not manually edit deployed hooks.
- Create .git-mem/ directory if it doesn't exist - Write config as YAML to .git-mem/.git-mem.yaml - Update .gitignore to ignore .git-mem/ directory - Parse existing config as YAML when merging 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-Decision: update init.ts to write YAML config (GIT-90). - Create .git-mem/ directory if it doesn't exist AI-Confidence: medium AI-Tags: commands, typescript AI-Lifecycle: project AI-Memory-Id: 4c2e5a59 AI-Source: heuristic
parseYaml() returns null for empty files. Add type guard to coerce null/non-object results to empty object before passing to deepMergeGitMemConfig(). Addresses CodeRabbit review feedback. 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: guard YAML parsing against null values. parseYaml() returns null for empty files. Add type guard to coerce AI-Confidence: medium AI-Tags: commands, typescript AI-Lifecycle: project AI-Memory-Id: 3d999379 AI-Source: heuristic
This was referenced Feb 15, 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
1 participant
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/directory if it doesn't exist.git-mem/.git-mem.yaml.gitignoreto ignore.git-mem/directoryCloses GIT-90
Test plan
npm run pre-commitpasses (type-check + lint)🤖 Generated with Claude Code
Summary by CodeRabbit