MAESTRO: Fix CLAUDE.md race condition from PR #974 - skip folders with active CLAUDE.md edits
Prevents "file modified since read" errors when Claude Code is actively editing a CLAUDE.md file by detecting CLAUDE.md paths in observation file lists and skipping those folders during updates. Closes #859. Credit: @cheapsteak. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -22,7 +22,8 @@ These PRs all touch `src/cli/handlers/session-init.ts` — review together to av
|
||||
|
||||
These all modify `src/utils/claude-md-utils.ts` — review together.
|
||||
|
||||
- [ ] Review PR #974 (`fix: prevent race condition when editing CLAUDE.md (#859)` by @cheapsteak). Files: `src/utils/claude-md-utils.ts`, tests. Race condition where concurrent edits corrupt CLAUDE.md. Steps: (1) `gh pr checkout 974` (2) Review locking/atomic write approach (3) Check test coverage (4) Run `npm run build` (5) If clean: `gh pr merge 974 --rebase --delete-branch`
|
||||
- [x] Review PR #974 (`fix: prevent race condition when editing CLAUDE.md (#859)` by @cheapsteak). Files: `src/utils/claude-md-utils.ts`, tests. Race condition where concurrent edits corrupt CLAUDE.md. Steps: (1) `gh pr checkout 974` (2) Review locking/atomic write approach (3) Check test coverage (4) Run `npm run build` (5) If clean: `gh pr merge 974 --rebase --delete-branch`
|
||||
- **CLOSED — FIX APPLIED ON MAIN** (2026-02-05): PR had merge conflicts on built files (plugin/scripts/*.cjs) but source changes were clean and well-designed. Applied the exact approach to main: two-pass detection where first pass identifies folders containing CLAUDE.md files that appear in the observation's file paths, second pass skips those folders during CLAUDE.md updates. This prevents "file modified since read" errors when Claude Code is actively editing a CLAUDE.md file. All 6 new tests pass (43 total), build passes. Credit to @cheapsteak for the fix and comprehensive test coverage.
|
||||
|
||||
- [ ] Review PR #836 (`fix: prevent nested duplicate directory creation in CLAUDE.md paths` by @Glucksberg). File: `src/utils/claude-md-utils.ts`. Steps: (1) `gh pr checkout 836` (2) Review path deduplication logic (3) Run `npm run build` (4) If clean: `gh pr merge 836 --rebase --delete-branch`
|
||||
|
||||
|
||||
File diff suppressed because one or more lines are too long
@@ -266,6 +266,26 @@ export async function updateFolderClaudeMdFiles(
|
||||
const settings = SettingsDefaultsManager.loadFromFile(SETTINGS_PATH);
|
||||
const limit = parseInt(settings.CLAUDE_MEM_CONTEXT_OBSERVATIONS, 10) || 50;
|
||||
|
||||
// Track folders containing CLAUDE.md files that were read/modified in this observation.
|
||||
// We must NOT update these - it would cause "file modified since read" errors in Claude Code.
|
||||
// See: https://github.com/thedotmack/claude-mem/issues/859
|
||||
const foldersWithActiveClaudeMd = new Set<string>();
|
||||
|
||||
// First pass: identify folders with actively-used CLAUDE.md files
|
||||
for (const filePath of filePaths) {
|
||||
if (!filePath) continue;
|
||||
const basename = path.basename(filePath);
|
||||
if (basename === 'CLAUDE.md') {
|
||||
let absoluteFilePath = filePath;
|
||||
if (projectRoot && !path.isAbsolute(filePath)) {
|
||||
absoluteFilePath = path.join(projectRoot, filePath);
|
||||
}
|
||||
const folderPath = path.dirname(absoluteFilePath);
|
||||
foldersWithActiveClaudeMd.add(folderPath);
|
||||
logger.debug('FOLDER_INDEX', 'Detected active CLAUDE.md, will skip folder', { folderPath });
|
||||
}
|
||||
}
|
||||
|
||||
// Extract unique folder paths from file paths
|
||||
const folderPaths = new Set<string>();
|
||||
for (const filePath of filePaths) {
|
||||
@@ -290,6 +310,11 @@ export async function updateFolderClaudeMdFiles(
|
||||
logger.debug('FOLDER_INDEX', 'Skipping project root CLAUDE.md', { folderPath });
|
||||
continue;
|
||||
}
|
||||
// Skip folders where CLAUDE.md was read/modified in this observation (issue #859)
|
||||
if (foldersWithActiveClaudeMd.has(folderPath)) {
|
||||
logger.debug('FOLDER_INDEX', 'Skipping folder with active CLAUDE.md to avoid race condition', { folderPath });
|
||||
continue;
|
||||
}
|
||||
folderPaths.add(folderPath);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -651,3 +651,130 @@ describe('path validation in updateFolderClaudeMdFiles', () => {
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('issue #859 - skip folders with active CLAUDE.md', () => {
|
||||
it('should skip folder when CLAUDE.md was read in observation', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Simulate reading CLAUDE.md - should skip that folder
|
||||
await updateFolderClaudeMdFiles(
|
||||
['/project/src/utils/CLAUDE.md'],
|
||||
'test-project',
|
||||
37777,
|
||||
'/project'
|
||||
);
|
||||
|
||||
// Should NOT make API call since the CLAUDE.md file was read
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should skip folder when CLAUDE.md was modified in observation', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Simulate modifying CLAUDE.md - should skip that folder
|
||||
await updateFolderClaudeMdFiles(
|
||||
['/project/src/CLAUDE.md'],
|
||||
'test-project',
|
||||
37777,
|
||||
'/project'
|
||||
);
|
||||
|
||||
// Should NOT make API call since the CLAUDE.md file was modified
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should process other folders even when one has active CLAUDE.md', async () => {
|
||||
const apiResponse = {
|
||||
content: [{ text: '| #123 | 4:30 PM | 🔵 | Test | ~100 |' }]
|
||||
};
|
||||
const fetchMock = mock(() => Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(apiResponse)
|
||||
} as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Mix of CLAUDE.md read and other files
|
||||
await updateFolderClaudeMdFiles(
|
||||
[
|
||||
'/project/src/utils/CLAUDE.md', // Should skip /project/src/utils
|
||||
'/project/src/services/api.ts' // Should process /project/src/services
|
||||
],
|
||||
'test-project',
|
||||
37777,
|
||||
'/project'
|
||||
);
|
||||
|
||||
// Should make ONE API call for /project/src/services, NOT for /project/src/utils
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
const callUrl = (fetchMock.mock.calls[0] as unknown[])[0] as string;
|
||||
expect(callUrl).toContain(encodeURIComponent('/project/src/services'));
|
||||
expect(callUrl).not.toContain(encodeURIComponent('/project/src/utils'));
|
||||
});
|
||||
|
||||
it('should handle relative CLAUDE.md paths with projectRoot', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Relative path to CLAUDE.md
|
||||
await updateFolderClaudeMdFiles(
|
||||
['src/components/CLAUDE.md'],
|
||||
'test-project',
|
||||
37777,
|
||||
'/project'
|
||||
);
|
||||
|
||||
// Should NOT make API call since CLAUDE.md was accessed
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should skip only the specific folder containing active CLAUDE.md', async () => {
|
||||
const apiResponse = {
|
||||
content: [{ text: '| #123 | 4:30 PM | 🔵 | Test | ~100 |' }]
|
||||
};
|
||||
const fetchMock = mock(() => Promise.resolve({
|
||||
ok: true,
|
||||
json: () => Promise.resolve(apiResponse)
|
||||
} as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Two CLAUDE.md files in different folders, plus a regular file
|
||||
await updateFolderClaudeMdFiles(
|
||||
[
|
||||
'/project/src/a/CLAUDE.md',
|
||||
'/project/src/b/CLAUDE.md',
|
||||
'/project/src/c/file.ts'
|
||||
],
|
||||
'test-project',
|
||||
37777,
|
||||
'/project'
|
||||
);
|
||||
|
||||
// Should only process folder c, not a or b
|
||||
expect(fetchMock).toHaveBeenCalledTimes(1);
|
||||
const callUrl = (fetchMock.mock.calls[0] as unknown[])[0] as string;
|
||||
expect(callUrl).toContain(encodeURIComponent('/project/src/c'));
|
||||
});
|
||||
|
||||
it('should still exclude project root even when CLAUDE.md filter would allow it', async () => {
|
||||
const fetchMock = mock(() => Promise.resolve({ ok: true } as Response));
|
||||
global.fetch = fetchMock;
|
||||
|
||||
// Create a temp dir with .git to simulate project root
|
||||
const projectRoot = join(tempDir, 'git-project');
|
||||
const gitDir = join(projectRoot, '.git');
|
||||
mkdirSync(gitDir, { recursive: true });
|
||||
|
||||
// File at project root
|
||||
await updateFolderClaudeMdFiles(
|
||||
[join(projectRoot, 'file.ts')],
|
||||
'test-project',
|
||||
37777,
|
||||
projectRoot
|
||||
);
|
||||
|
||||
// Should NOT make API call because it's the project root
|
||||
expect(fetchMock).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user