Skip to content

fix: handle undefined content in memory query (GIT-96) #71

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TonyCasey merged 4 commits into from
Feb 15, 2026
Merged

fix: handle undefined content in memory query (GIT-96) #71

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
eb9875c
fix: handle undefined content in memory query (GIT-96)
TonyCasey Feb 15, 2026
ad93729
fix: guard tag filter and strengthen regression tests
TonyCasey Feb 15, 2026
720533f
feat: add complete hook config to init command
TonyCasey Feb 15, 2026
7a5d725
fix: update console output for new hook defaults
TonyCasey Feb 15, 2026
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
7 changes: 5 additions & 2 deletions src/application/services/MemoryService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,9 @@ export class MemoryService implements IMemoryService {
const type = TRAILER_KEY_TO_MEMORY_TYPE[typeTrailer.key] as MemoryType;
if (!type) continue;

// Skip trailers with empty or undefined values
if (!typeTrailer.value) continue;

// Pair with AI-Memory-Id by position, or generate synthetic ID.
// Index suffix ensures uniqueness when a commit has multiple same-type trailers.
const id = memoryIds[i] || `trailer:${commit.sha}:${type}:${i}`;
Expand All @@ -232,8 +235,8 @@ export class MemoryService implements IMemoryService {

private matchesQuery(entity: IMemoryEntity, query: string): boolean {
const lower = query.toLowerCase();
return entity.content.toLowerCase().includes(lower) ||
entity.tags.some(t => t.toLowerCase().includes(lower));
return (entity.content?.toLowerCase().includes(lower) ?? false) ||
(entity.tags?.some(t => t?.toLowerCase().includes(lower)) ?? false);
}

get(id: string, cwd?: string): IMemoryEntity | null {
Expand Down
19 changes: 17 additions & 2 deletions src/commands/init-hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,13 +202,27 @@ export function buildGitMemConfig(): Record<string, unknown> {
threshold: 3,
},
promptSubmit: {
enabled: false,
enabled: true,
recordPrompts: false,
surfaceContext: true,
extractIntent: true,
intentTimeout: 3000,
minWords: 5,
memoryLimit: 20,
includeCommitMessages: true,
},
Comment thread
coderabbitai[bot] marked this conversation as resolved.
postCommit: {
enabled: true,
},
commitMsg: {
enabled: true,
autoAnalyze: true,
inferTags: true,
requireType: false,
defaultLifecycle: 'project',
enrich: true,
enrichTimeout: 8000,
},
Comment on lines +205 to +225
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The changes to init-hooks.ts appear unrelated to the bug fix described in the PR (handling undefined content in memory queries). This change modifies the default configuration to enable promptSubmit and add new config fields, as well as adding a complete commitMsg config block. These configuration changes should be in a separate PR with appropriate documentation and testing, as they change the default behavior of the tool and are not related to fixing the GIT-96 bug.

Copilot uses AI. Check for mistakes.
Comment on lines +217 to +225
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 15, 2026

Choose a reason for hiding this comment

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

🧹 Nitpick | 🔵 Trivial

New commitMsg hook not documented in console output.

The commitMsg hook block is added to the default configuration, but the "Hooks configured" summary (lines 307-310) does not mention it. Consider adding a line describing this hook so users know it's active.

📝 Suggested addition 
   console.log('  SessionStart     — Load memories into Claude context on startup');
   console.log('  Stop             — Capture memories from session commits on exit');
   console.log('  UserPromptSubmit — Surface relevant memories per prompt (disabled by default)');
+  console.log('  CommitMsg        — Analyze and enrich commit messages');
🤖 Prompt for AI Agents 
In `@src/commands/init-hooks.ts` around lines 217 - 225, The new commitMsg
configuration block (commitMsg: { enabled: true, autoAnalyze: true, inferTags:
true, requireType: false, defaultLifecycle: 'project', enrich: true,
enrichTimeout: 8000 }) was added but not listed in the "Hooks configured"
console summary; update the code that builds/renders that summary to include an
entry for commitMsg (showing at least enabled state and key flags like
autoAnalyze/enrich) so users see it active—locate the summary/printing logic
that outputs the "Hooks configured" lines and add commitMsg to the displayed
hooks.

},
};
}
Expand Down Expand Up @@ -293,7 +307,8 @@ export async function initHooksCommand(options: IInitHooksOptions, logger?: ILog
console.log('\nHooks configured:');
console.log(' SessionStart — Load memories into Claude context on startup');
console.log(' Stop — Capture memories from session commits on exit');
console.log(' UserPromptSubmit — Surface relevant memories per prompt (disabled by default)');
console.log(' UserPromptSubmit — Surface relevant memories per prompt');
console.log(' CommitMsg — Analyze commits and add AI trailers');

console.log('\nNext steps:');
console.log(' 1. Start Claude Code in this repo: claude');
Expand Down
6 changes: 3 additions & 3 deletions src/infrastructure/repositories/MemoryRepository.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class MemoryRepository implements IMemoryRepository {
}

if (options?.tag) {
filtered = filtered.filter(m => m.tags.includes(options.tag!));
filtered = filtered.filter(m => m.tags?.includes(options.tag!) ?? false);
}

if (options?.since) {
Expand All @@ -96,8 +96,8 @@ export class MemoryRepository implements IMemoryRepository {
if (options?.query) {
const q = options.query.toLowerCase();
filtered = filtered.filter(m =>
m.content.toLowerCase().includes(q) ||
m.tags.some(t => t.toLowerCase().includes(q))
(m.content?.toLowerCase().includes(q) ?? false) ||
(m.tags?.some(t => t?.toLowerCase().includes(q)) ?? false)
);
Comment on lines 96 to 101
Copy link

Copilot AI Feb 15, 2026

Choose a reason for hiding this comment

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

The optional-chaining here prevents crashes for malformed notes during query-text filtering, but options.tag filtering earlier still calls m.tags.includes(...) and can throw if a legacy/malformed note omits tags (which is plausible when parsing unvalidated JSON). Consider normalizing parsed memories in readPayload (default tags to [], content to '') or guarding tag filtering similarly so mcp:recall with tag can’t crash on malformed data.

Copilot uses AI. Check for mistakes.
}

Expand Down
30 changes: 30 additions & 0 deletions tests/unit/application/services/MemoryService.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,36 @@ describe('MemoryService', () => {
const result = service.recall('nonexistent-xyz-query', { cwd: repoDir });
assert.equal(result.memories.length, 0);
});

it('should handle query on memories with undefined content gracefully (GIT-96)', () => {
// Regression test: inject actual malformed data with missing content/tags
const malformedFile = join(repoDir, 'git-96-memsvc-malformed.txt');
writeFileSync(malformedFile, 'malformed memory service test');
git(['add', 'git-96-memsvc-malformed.txt'], repoDir);
git(['commit', '-m', 'git-96 memsvc malformed memory'], repoDir);
const malformedSha = git(['rev-parse', 'HEAD'], repoDir);

// Write a malformed note payload (missing content and tags fields)
const malformedPayload = JSON.stringify({
memories: [{
id: 'git-96-memsvc-malformed-id',
type: 'gotcha',
sha: malformedSha,
confidence: 'medium',
source: 'user-explicit',
lifecycle: 'project',
// Intentionally omit content and tags to simulate malformed data
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}],
});

git(['notes', '--ref', 'refs/notes/mem', 'add', '-f', '-m', malformedPayload, malformedSha], repoDir);

// Query should not throw even with malformed data
const result = service.recall('test-query', { cwd: repoDir });
assert.ok(Array.isArray(result.memories));
});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Comment on lines +202 to +230
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

This unit test uses real git operations; prefer mocks or move to integration tests.

The new regression test relies on git commands and notes, which violates the unit-test isolation requirement. Consider mocking NotesService/MemoryRepository to inject malformed data, or relocate to tests/integration/.

As per coding guidelines: "Unit tests must be fast, isolated, and mock all dependencies. Integration tests should use real backends and test contracts."

🤖 Prompt for AI Agents 
In `@tests/unit/application/services/MemoryService.test.ts` around lines 202 -
230, The test in MemoryService.test.ts performs real git operations (git, git
notes, repoDir, service.recall) which breaks unit-test isolation; change it to
inject malformed data by mocking the NotesService or MemoryRepository used by
MemoryService (or replace calls that create git notes/commits with a
stubbed/mocked method that returns the malformed payload) so
service.recall('test-query', { cwd: repoDir }) receives the malformed memory
object without invoking the git binary; alternatively, move this scenario into
an integration test under tests/integration/ if you need real git behavior.

});

describe('unified recall (notes + trailers)', () => {
Expand Down
35 changes: 35 additions & 0 deletions tests/unit/infrastructure/repositories/MemoryRepository.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,41 @@ describe('MemoryRepository', () => {
const result = repo.query({ limit: 1, cwd: repoDir });
assert.ok(result.memories.length <= 1);
});

it('should handle query on memories with undefined content gracefully (GIT-96)', () => {
// Regression test: inject actual malformed data with missing content/tags
// to verify the defensive guards work
const malformedFile = join(repoDir, 'git-96-malformed.txt');
writeFileSync(malformedFile, 'malformed memory test');
git(['add', 'git-96-malformed.txt'], repoDir);
git(['commit', '-m', 'git-96 malformed memory'], repoDir);
const malformedSha = git(['rev-parse', 'HEAD'], repoDir);

// Write a malformed note payload (missing content and tags fields)
const malformedPayload = JSON.stringify({
memories: [{
id: 'git-96-malformed-id',
type: 'decision',
sha: malformedSha,
confidence: 'high',
source: 'user-explicit',
lifecycle: 'project',
// Intentionally omit content and tags to simulate malformed data
createdAt: new Date().toISOString(),
updatedAt: new Date().toISOString(),
}],
});

git(['notes', '--ref', 'refs/notes/mem', 'add', '-f', '-m', malformedPayload, malformedSha], repoDir);

// Query with text search should not throw
const queryResult = repo.query({ query: 'test-search', cwd: repoDir });
assert.ok(Array.isArray(queryResult.memories));

// Query with tag filter should not throw
const tagResult = repo.query({ tag: 'some-tag', cwd: repoDir });
assert.ok(Array.isArray(tagResult.memories));
Comment on lines +123 to +155
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Feb 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Unit test should not depend on real git operations.

This regression test uses real git commands and notes, which conflicts with the unit-test isolation requirement. Prefer mocking NotesService to supply malformed payloads, or move this case to tests/integration/.

As per coding guidelines: "Unit tests must be fast, isolated, and mock all dependencies. Integration tests should use real backends and test contracts."

🤖 Prompt for AI Agents 
In `@tests/unit/infrastructure/repositories/MemoryRepository.test.ts` around lines
123 - 155, This unit test improperly performs real git operations; replace the
live git/notes manipulation with a mocked NotesService response to keep the test
isolated: remove calls to git(...) and git notes/commit setup, create a fake
malformed payload (missing content/tags) and stub the NotesService (or the
method MemoryRepository uses to read notes) to return that payload when
MemoryRepository.query or the repository's internal notes-fetching function is
invoked, then call repo.query({ query: 'test-search', cwd: repoDir }) and
repo.query({ tag: 'some-tag', cwd: repoDir }) and assert they return arrays as
before; target the test harness around MemoryRepository/NotesService and the
repo.query call to inject the mock rather than touching git.

});
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

describe('delete', () => {
Expand Down