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
56 changes: 56 additions & 0 deletions packages/core/src/memory/extract.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ describe('auto-memory extraction', () => {
touchedTopics: [],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -125,6 +126,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});
vi.mocked(rebuildManagedAutoMemoryIndex).mockRejectedValueOnce(
Expand All @@ -151,6 +153,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: true,
hasToolActivity: true,
systemMessage: undefined,
});
vi.mocked(rebuildManagedAutoMemoryIndex).mockResolvedValueOnce('');
Expand Down Expand Up @@ -180,6 +183,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user', 'project'],
touchedProjectScope: true,
touchedUserScope: true,
hasToolActivity: true,
systemMessage: undefined,
});

Expand All @@ -200,6 +204,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -227,6 +232,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -284,6 +290,7 @@ describe('auto-memory extraction', () => {
touchedTopics: [],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -331,6 +338,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand All @@ -356,6 +364,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -400,6 +409,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -489,6 +499,7 @@ describe('auto-memory extraction', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

Expand Down Expand Up @@ -532,5 +543,50 @@ describe('auto-memory extraction', () => {
);
expect(result.cursor.processedOffset).toBe(compressedHistory.length);
});
it('BUG #6311: should NOT advance cursor when agent makes zero tool calls (hallucination)', async () => {
vi.mocked(runAutoMemoryExtractionByAgent).mockResolvedValue({
touchedTopics: [],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: false,
systemMessage: undefined,
});

const history = [
{
role: 'user' as const,
parts: [{ text: 'Remember that I prefer pnpm over npm.' }],
},
];

const result = await runAutoMemoryExtract({
projectRoot,
sessionId: 'session-1',
config: mockConfig,
history: [...history],
});

expect(result.cursor.processedOffset).toBe(0);
});
it('should advance cursor on legitimate noop (agent checked memory, found nothing new)', async () => {
vi.mocked(runAutoMemoryExtractionByAgent).mockResolvedValue({
touchedTopics: [],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
});

const history = [{ role: 'user' as const, parts: [{ text: 'hello' }] }];

const result = await runAutoMemoryExtract({
projectRoot,
sessionId: 'session-1',
config: mockConfig,
history: [...history],
});

expect(result.cursor.processedOffset).toBe(1);
});
});
});
5 changes: 4 additions & 1 deletion packages/core/src/memory/extract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,12 @@ export async function runAutoMemoryExtract(params: {
await Promise.all([projectRebuild, userRebuild]);
}

const madeGenuineProgress =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] touchedTopics.length > 0 is redundant with hasToolActivity. Since touchedTopics is derived from filesWritten (a strict subset of filesTouched — see forkedAgent.ts lines 568→582), touchedTopics.length > 0 always implies hasToolActivity is already true. The left side of the || can never be the deciding factor.

This could mislead future maintainers into thinking there are two independent paths to cursor advancement. Either simplify:

Suggested change
const madeGenuineProgress =
const madeGenuineProgress = agentResult.hasToolActivity;

Or add a comment explaining the defensive intent:

Suggested change
const madeGenuineProgress =
// touchedTopics.length > 0 implies hasToolActivity today (filesWritten ⊆ filesTouched),
// but the explicit check guards against a future planner refactor that decouples them.
const madeGenuineProgress =
agentResult.touchedTopics.length > 0 || agentResult.hasToolActivity;

— qwen3.7-max via Qwen Code /review

agentResult.touchedTopics.length > 0 || agentResult.hasToolActivity;

const cursor: AutoMemoryExtractCursor = {
sessionId: params.sessionId,
processedOffset: params.history.length,
processedOffset: madeGenuineProgress ? params.history.length : startOffset,
updatedAt: now.toISOString(),
};
await writeExtractCursor(params.projectRoot, cursor);
Expand Down
1 change: 1 addition & 0 deletions packages/core/src/memory/extractAgent.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ describe('auto-memory extraction with agent planner', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: 'Managed auto-memory updated: user.md',
};
});
Expand Down
25 changes: 25 additions & 0 deletions packages/core/src/memory/extractionAgentPlanner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ describe('runAutoMemoryExtractionByAgent', () => {
status: 'completed',
finalText: '',
filesTouched: ['/tmp/auto-memory/user/prefs.md'],
filesWritten: ['/tmp/auto-memory/user/prefs.md'],

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] All tests in this file set filesTouched and filesWritten to identical arrays. No test exercises the scenario where they diverge (e.g., agent reads a memory file but decides not to write to it). The PR's core semantic split — filesWritten for topic classification, filesTouched for hasToolActivity — is unverified at the unit level.

Consider adding a test like:

it('derives touchedTopics from filesWritten, not filesTouched', async () => {
  vi.mocked(runForkedAgent).mockResolvedValue({
    status: 'completed',
    finalText: '',
    filesTouched: ['/tmp/auto-memory/user/prefs.md', '/tmp/auto-memory/user/readonly.md'],
    filesWritten: ['/tmp/auto-memory/user/prefs.md'],
  });
  const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
  expect(result.touchedTopics).toEqual(['user']);
  expect(result.hasToolActivity).toBe(true);
});

This proves touchedTopics reflects only filesWritten while hasToolActivity reflects filesTouched.

— qwen3.7-max via Qwen Code /review

});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
Expand All @@ -84,6 +85,7 @@ describe('runAutoMemoryExtractionByAgent', () => {
touchedTopics: ['user'],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: 'Managed auto-memory updated: user.md',
});
expect(runForkedAgent).toHaveBeenCalledWith(
Expand All @@ -108,13 +110,15 @@ describe('runAutoMemoryExtractionByAgent', () => {
status: 'completed',
finalText: '',
filesTouched: [],
filesWritten: [],
});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
expect(result).toEqual({
touchedTopics: [],
touchedProjectScope: false,
touchedUserScope: false,
hasToolActivity: false,
systemMessage: undefined,
});
});
Expand Down Expand Up @@ -176,6 +180,11 @@ describe('runAutoMemoryExtractionByAgent', () => {
'/tmp/auto-memory/reference/api.md',
'/tmp/some/other/file.ts',
],
filesWritten: [
'/tmp/auto-memory/project/arch.md',
'/tmp/auto-memory/reference/api.md',
'/tmp/some/other/file.ts',
],
});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
Expand All @@ -195,6 +204,10 @@ describe('runAutoMemoryExtractionByAgent', () => {
'/tmp/user-memory/user/role.md',
'/tmp/user-memory/feedback/terse.md',
],
filesWritten: [
'/tmp/user-memory/user/role.md',
'/tmp/user-memory/feedback/terse.md',
],
});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
Expand Down Expand Up @@ -230,6 +243,10 @@ describe('runAutoMemoryExtractionByAgent', () => {
'C:/Users/foo/.qwen/projects/proj/memory/project/release.md',
'C:/Users/foo/.qwen/memories/user/role.md',
],
filesWritten: [
'C:/Users/foo/.qwen/projects/proj/memory/project/release.md',
'C:/Users/foo/.qwen/memories/user/role.md',
],
});

try {
Expand All @@ -256,6 +273,10 @@ describe('runAutoMemoryExtractionByAgent', () => {
'/tmp/auto-memory\\project\\arch.md',
'/tmp/user-memory\\user\\role.md',
],
filesWritten: [
'/tmp/auto-memory\\project\\arch.md',
'/tmp/user-memory\\user\\role.md',
],
});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
Expand Down Expand Up @@ -294,6 +315,10 @@ describe('runAutoMemoryExtractionByAgent', () => {
'/tmp/user-memory/user/role.md',
'/tmp/auto-memory/project/release.md',
],
filesWritten: [
'/tmp/user-memory/user/role.md',
'/tmp/auto-memory/project/release.md',
],
});

const result = await runAutoMemoryExtractionByAgent(mockConfig, '/tmp');
Expand Down
4 changes: 3 additions & 1 deletion packages/core/src/memory/extractionAgentPlanner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ export interface AutoMemoryExtractionExecutionResult {
/** True when at least one file inside the user-level memory root was written/edited. */
touchedUserScope: boolean;
systemMessage?: string;
hasToolActivity: boolean;
}

/**
Expand Down Expand Up @@ -291,12 +292,13 @@ export async function runAutoMemoryExtractionByAgent(
}

const { topics, touchedProjectScope, touchedUserScope } =
touchedTopicsFromFilePaths(result.filesTouched, projectRoot);
touchedTopicsFromFilePaths(result.filesWritten ?? [], projectRoot);

return {
touchedTopics: topics,
touchedProjectScope,
touchedUserScope,
hasToolActivity: result.filesTouched.length > 0,
systemMessage:
topics.length > 0
? `Managed auto-memory updated: ${topics.map((t) => `${t}.md`).join(', ')}`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ describe('managed auto-memory lifecycle integration', () => {
touchedTopics: [topic],
touchedProjectScope: true,
touchedUserScope: false,
hasToolActivity: true,
systemMessage: undefined,
};
},
Expand Down
Loading