From cfe3d874072f75f43138791d6fab1ba9e370aad8 Mon Sep 17 00:00:00 2001 From: Prashanth Subrahmanyam Date: Sun, 5 Oct 2025 10:27:06 +0530 Subject: [PATCH] feat(core): improve import processor regex This commit improves the import processor regex to avoid matching code inside markdown code blocks. The new logic is based on the following rules: - A path is valid if it starts with `./`, `../`, or `/`. - A path is valid if it has a file extension. This change also includes new tests to cover the following cases: - npm-style packages in code blocks - Python decorators - Email addresses - Decorators - Subdirectory imports with and without extensions Fixes #2967 --- .../src/utils/memoryImportProcessor.test.ts | 170 ++++++++++++++++-- .../core/src/utils/memoryImportProcessor.ts | 27 ++- 2 files changed, 166 insertions(+), 31 deletions(-) diff --git a/packages/core/src/utils/memoryImportProcessor.test.ts b/packages/core/src/utils/memoryImportProcessor.test.ts index 0fcaf080d3d..d7f871f73da 100644 --- a/packages/core/src/utils/memoryImportProcessor.test.ts +++ b/packages/core/src/utils/memoryImportProcessor.test.ts @@ -180,27 +180,165 @@ describe('memoryImportProcessor', () => { ); }); - it('should handle circular imports', async () => { - const content = 'Content @./circular.md more content'; - const basePath = testPath('test', 'path'); - const circularContent = 'Circular @./main.md content'; + it('should handle circular imports gracefully', async () => { + const content = '@a.md'; + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + // Setup circular import: a.md -> b.md -> a.md mockedFs.access.mockResolvedValue(undefined); - mockedFs.readFile.mockResolvedValue(circularContent); + mockedFs.readFile.mockImplementation(async (p) => { + const filePath = p as string; + if (filePath.endsWith('a.md')) { + return '@b.md'; + } + if (filePath.endsWith('b.md')) { + return '@a.md'; + } + return ''; + }); - // Set up the import state to simulate we're already processing main.md - const importState = { - processedFiles: new Set(), - maxDepth: 10, - currentDepth: 0, - currentFile: testPath('test', 'path', 'main.md'), // Simulate we're processing main.md - }; + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); - const result = await processImports(content, basePath, true, importState); + expect(result.content).toContain(''); + }); - // The circular import should be detected when processing the nested import - expect(result.content).toContain( - '', + it('should not treat npm-style packages in code blocks as imports', async () => { + const content = [ + '* **Frontend:** `vitest` is used for testing. Run with `pnpm -F @google-cloud-pulse/frontend test`.', + '* **Backend:** `jest` is used for testing. Run with `pnpm -F @google-cloud-pulse/backend test`.', + ].join('\n'); + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); + + // No imports should be processed + expect(mockedFs.readFile).not.toHaveBeenCalled(); + + // The original content should be unchanged + expect(result.content).toBe(content); + + // No error messages should be logged + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should not treat python decorators in code blocks as imports', async () => { + const content = [ + '```python', + '@app.route("/api")', + 'def my_api():', + ' return "OK",', + '```', + ].join('\n'); + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); + + // No imports should be processed + expect(mockedFs.readFile).not.toHaveBeenCalled(); + + // The original content should be unchanged + expect(result.content).toBe(content); + + // No error messages should be logged + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should not treat email addresses as imports', async () => { + const content = 'Contact us at contact@example.com'; + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); + + // No imports should be processed + expect(mockedFs.readFile).not.toHaveBeenCalled(); + + // The original content should be unchanged + expect(result.content).toBe(content); + + // No error messages should be logged + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should not treat decorators as imports', async () => { + const content = 'This is a decorator @my-decorator'; + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); + + // No imports should be processed + expect(mockedFs.readFile).not.toHaveBeenCalled(); + + // The original content should be unchanged + expect(result.content).toBe(content); + + // No error messages should be logged + expect(console.error).not.toHaveBeenCalled(); + }); + + it('should handle subdirectory imports with extensions but ignore those without', async () => { + const content = + 'Import with extension: @foo/bar.md and without: @foo/bar'; + const projectRoot = testPath('test', 'project'); + const basePath = testPath(projectRoot, 'src'); + const importedContent = 'Subdirectory content'; + + mockedFs.access.mockResolvedValue(undefined); + mockedFs.readFile.mockResolvedValueOnce(importedContent); + + const result = await processImports( + content, + basePath, + true, + undefined, + projectRoot, + ); + + // Verify the valid import was processed + expect(result.content).toContain(importedContent); + expect(result.content).toContain(''); + + // Verify the invalid import was ignored + expect(result.content).toContain('@foo/bar'); + expect(result.content).not.toContain(''); + expect(mockedFs.readFile).toHaveBeenCalledTimes(1); + expect(mockedFs.readFile).toHaveBeenCalledWith( + path.resolve(basePath, 'foo/bar.md'), + 'utf-8', ); }); diff --git a/packages/core/src/utils/memoryImportProcessor.ts b/packages/core/src/utils/memoryImportProcessor.ts index 4af71cdfd8e..6d4cfef2ace 100644 --- a/packages/core/src/utils/memoryImportProcessor.ts +++ b/packages/core/src/utils/memoryImportProcessor.ts @@ -118,13 +118,18 @@ function findImports( // Extract the path (everything after @) const importPath = content.slice(i + 1, j); - // Basic validation (starts with ./ or / or letter) - if ( - importPath.length > 0 && - (importPath[0] === '.' || - importPath[0] === '/' || - isLetter(importPath[0])) - ) { + // A valid import path is one of the following: + // 1. An explicit relative or absolute path (eg: ./.gemini/FILE.md). + // 2. A path with or wihout slashes that has a file extension in the final segment + // (e.g., @folder/file.md), to distinguish from npm packages. + const hasExtension = path.basename(importPath).includes('.'); + const isValid = + importPath.startsWith('./') || + importPath.startsWith('../') || + importPath.startsWith('/') || + hasExtension; + + if (importPath.length > 0 && isValid) { imports.push({ start: i, _end: j, @@ -142,14 +147,6 @@ function isWhitespace(char: string): boolean { return char === ' ' || char === '\t' || char === '\n' || char === '\r'; } -function isLetter(char: string): boolean { - const code = char.charCodeAt(0); - return ( - (code >= 65 && code <= 90) || // A-Z - (code >= 97 && code <= 122) - ); // a-z -} - function findCodeRegions(content: string): Array<[number, number]> { const regions: Array<[number, number]> = []; const tokens = marked.lexer(content);