diff --git a/src/frameworks/python/python-wizard-agent.ts b/src/frameworks/python/python-wizard-agent.ts index fc44b370b..8e1468c73 100644 --- a/src/frameworks/python/python-wizard-agent.ts +++ b/src/frameworks/python/python-wizard-agent.ts @@ -4,7 +4,7 @@ import type { FrameworkConfig } from '@lib/framework-config'; import { PYTHON_PACKAGE_INSTALLATION } from '@lib/framework-config'; import { detectPythonPackageManagers } from '@lib/detection/package-manager'; import { Integration } from '@lib/constants'; -import fg from 'fast-glob'; +import { DETECTION_IGNORE_PATTERNS, globWithAbort } from '@lib/detection/glob'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { @@ -40,10 +40,10 @@ export const PYTHON_AGENT_CONFIG: FrameworkConfig = { getInstalledVersion: (options: WizardRunOptions) => Promise.resolve(getPythonVersion(options)), detect: async (options) => { - const { installDir } = options; + const { installDir, signal } = options; // Look for Python package management files - const pythonConfigFiles = await fg( + const pythonConfigFiles = await globWithAbort( [ '**/requirements*.txt', '**/pyproject.toml', @@ -52,7 +52,8 @@ export const PYTHON_AGENT_CONFIG: FrameworkConfig = { ], { cwd: installDir, - ignore: ['**/venv/**', '**/.venv/**', '**/env/**', '**/.env/**'], + ignore: DETECTION_IGNORE_PATTERNS, + signal, }, ); @@ -62,9 +63,10 @@ export const PYTHON_AGENT_CONFIG: FrameworkConfig = { // Make sure this isn't Django or Flask (those should be detected first) // Check for Django - const managePyMatches = await fg('**/manage.py', { + const managePyMatches = await globWithAbort('**/manage.py', { cwd: installDir, - ignore: ['**/venv/**', '**/.venv/**', '**/env/**', '**/.env/**'], + ignore: DETECTION_IGNORE_PATTERNS, + signal, }); for (const match of managePyMatches) { @@ -102,17 +104,12 @@ export const PYTHON_AGENT_CONFIG: FrameworkConfig = { } } - const pyFiles = await fg( + const pyFiles = await globWithAbort( ['**/app.py', '**/wsgi.py', '**/application.py', '**/__init__.py'], { cwd: installDir, - ignore: [ - '**/venv/**', - '**/.venv/**', - '**/env/**', - '**/.env/**', - '**/__pycache__/**', - ], + ignore: DETECTION_IGNORE_PATTERNS, + signal, }, ); diff --git a/src/frameworks/swift/swift-wizard-agent.ts b/src/frameworks/swift/swift-wizard-agent.ts index 691554c80..1e7dcfaf7 100644 --- a/src/frameworks/swift/swift-wizard-agent.ts +++ b/src/frameworks/swift/swift-wizard-agent.ts @@ -3,12 +3,14 @@ import type { WizardRunOptions } from '@utils/types'; import type { FrameworkConfig } from '@lib/framework-config'; import { swiftPackageManager } from '@lib/detection/package-manager'; import { Integration } from '@lib/constants'; +import { globWithAbort } from '@lib/detection/glob'; import fg from 'fast-glob'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { detectSwiftProjectType, getSwiftProjectTypeName, + SWIFT_SOURCE_IGNORE_PATTERNS, SwiftProjectType, } from './utils'; @@ -35,10 +37,10 @@ export const SWIFT_AGENT_CONFIG: FrameworkConfig = { usesPackageJson: false, getVersion: () => undefined, detect: async (options) => { - const { installDir } = options; + const { installDir, signal } = options; // Xcode project/workspace, or an XcodeGen `project.yml` (the generated - // .xcodeproj is often uncommitted). + // .xcodeproj is often uncommitted). Non-recursive, so no deep walk. const xcodeProjects = await fg('*.{xcodeproj,xcworkspace}', { cwd: installDir, onlyDirectories: true, @@ -49,15 +51,10 @@ export const SWIFT_AGENT_CONFIG: FrameworkConfig = { if (xcodeProjects.length > 0 || hasXcodeGenSpec) { // Verify it contains Swift source files - const swiftFiles = await fg('**/*.swift', { + const swiftFiles = await globWithAbort('**/*.swift', { cwd: installDir, - ignore: [ - '**/.build/**', - '**/DerivedData/**', - '**/build/**', - '**/*.xcodeproj/**', - '**/Pods/**', - ], + ignore: SWIFT_SOURCE_IGNORE_PATTERNS, + signal, }); if (swiftFiles.length > 0) { return true; diff --git a/src/frameworks/swift/utils.ts b/src/frameworks/swift/utils.ts index 05ced9957..804af6f10 100644 --- a/src/frameworks/swift/utils.ts +++ b/src/frameworks/swift/utils.ts @@ -1,8 +1,22 @@ import type { WizardRunOptions } from '@utils/types'; +import { DETECTION_IGNORE_PATTERNS } from '@lib/detection/glob'; import fg from 'fast-glob'; import * as fs from 'node:fs'; import * as path from 'node:path'; +/** + * Ignore list for recursive Swift source walks: the shared heavy-tree patterns + * (node_modules, .git, build, …) plus Swift/Xcode build artifact directories. + * Without node_modules/.git these globs OOM on large or hybrid repos. + */ +export const SWIFT_SOURCE_IGNORE_PATTERNS = [ + ...DETECTION_IGNORE_PATTERNS, + '**/.build/**', + '**/DerivedData/**', + '**/*.xcodeproj/**', + '**/Pods/**', +]; + export enum SwiftProjectType { SWIFTUI = 'swiftui', UIKIT = 'uikit', @@ -42,13 +56,7 @@ export async function detectSwiftProjectType( // Check Swift source files for SwiftUI vs UIKit imports const swiftFiles = await fg('**/*.swift', { cwd: installDir, - ignore: [ - '**/.build/**', - '**/DerivedData/**', - '**/build/**', - '**/*.xcodeproj/**', - '**/Pods/**', - ], + ignore: SWIFT_SOURCE_IGNORE_PATTERNS, }); let hasSwiftUI = false; diff --git a/src/lib/cloudflare-detection.ts b/src/lib/cloudflare-detection.ts index 7dfe304b8..d49bbed79 100644 --- a/src/lib/cloudflare-detection.ts +++ b/src/lib/cloudflare-detection.ts @@ -1,6 +1,7 @@ import fg from 'fast-glob'; import { logToFile } from '@utils/debug'; import { tryGetPackageJson } from '@utils/setup-utils'; +import { DETECTION_IGNORE_PATTERNS } from '@lib/detection/glob'; const CLOUDFLARE_PACKAGES = [ '@react-router/cloudflare', @@ -22,10 +23,13 @@ const CLOUDFLARE_PACKAGES = [ export async function detectCloudflareTarget( installDir: string, ): Promise { - // Check for wrangler config files + // Check for wrangler config files. This pattern is root-only (no `**`), but + // we still pass the shared ignore list so it stays consistent with the other + // detectors and never descends into node_modules/.git if it's ever widened. const wranglerFiles = await fg('wrangler.@(toml|jsonc|json)', { cwd: installDir, dot: true, + ignore: DETECTION_IGNORE_PATTERNS, }); if (wranglerFiles.length > 0) { logToFile( diff --git a/src/lib/detection/__tests__/glob.test.ts b/src/lib/detection/__tests__/glob.test.ts new file mode 100644 index 000000000..fe42a9b3a --- /dev/null +++ b/src/lib/detection/__tests__/glob.test.ts @@ -0,0 +1,104 @@ +import * as fs from 'node:fs'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { DETECTION_IGNORE_PATTERNS, globWithAbort } from '@lib/detection/glob'; +import { PYTHON_AGENT_CONFIG } from '../../../frameworks/python/python-wizard-agent'; +import { SWIFT_AGENT_CONFIG } from '../../../frameworks/swift/swift-wizard-agent'; + +const tmpDirs: string[] = []; +function project(files: Record): string { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'wizard-glob-')); + for (const [rel, content] of Object.entries(files)) { + const abs = path.join(dir, rel); + fs.mkdirSync(path.dirname(abs), { recursive: true }); + fs.writeFileSync(abs, content); + } + tmpDirs.push(dir); + return dir; +} + +afterAll(() => { + for (const dir of tmpDirs) { + fs.rmSync(dir, { recursive: true, force: true }); + } +}); + +describe('DETECTION_IGNORE_PATTERNS', () => { + test('ignores the heavy trees that OOM recursive detection globs', () => { + expect(DETECTION_IGNORE_PATTERNS).toContain('**/node_modules/**'); + expect(DETECTION_IGNORE_PATTERNS).toContain('**/.git/**'); + }); +}); + +describe('globWithAbort', () => { + test('behaves like fast-glob when no signal is provided', async () => { + const dir = project({ 'a.txt': '', 'sub/b.txt': '' }); + const matches = await globWithAbort('**/*.txt', { cwd: dir }); + expect(matches.sort()).toEqual(['a.txt', 'sub/b.txt']); + }); + + test('honours the ignore list', async () => { + const dir = project({ + 'keep.txt': '', + 'node_modules/dep/skip.txt': '', + }); + const matches = await globWithAbort('**/*.txt', { + cwd: dir, + ignore: DETECTION_IGNORE_PATTERNS, + }); + expect(matches).toEqual(['keep.txt']); + }); + + test('resolves immediately with no matches when the signal is already aborted', async () => { + const dir = project({ 'a.txt': '' }); + const controller = new AbortController(); + controller.abort(); + await expect( + globWithAbort('**/*.txt', { cwd: dir, signal: controller.signal }), + ).resolves.toEqual([]); + }); + + test('resolves (does not hang or throw) when aborted mid-walk', async () => { + const dir = project({ 'a.txt': '', 'sub/b.txt': '' }); + const controller = new AbortController(); + const promise = globWithAbort('**/*.txt', { + cwd: dir, + signal: controller.signal, + }); + controller.abort(); + await expect(promise).resolves.toBeInstanceOf(Array); + }); +}); + +describe('detectors ignore node_modules (regression: framework-detection OOM)', () => { + test('Python is not detected from config files buried in node_modules', async () => { + // A JS project with a Python-based dependency vendored in node_modules must + // not be mistaken for a Python project — and, more importantly, detection + // must never walk into node_modules to find out. + const dir = project({ + 'package.json': JSON.stringify({ dependencies: {} }), + 'node_modules/some-dep/requirements.txt': 'requests==2.0.0', + 'node_modules/some-dep/setup.py': 'from setuptools import setup', + }); + await expect( + PYTHON_AGENT_CONFIG.detection.detect({ installDir: dir }), + ).resolves.toBe(false); + }); + + test('Python is still detected from real project config files', async () => { + const dir = project({ 'requirements.txt': 'requests==2.0.0' }); + await expect( + PYTHON_AGENT_CONFIG.detection.detect({ installDir: dir }), + ).resolves.toBe(true); + }); + + test('Swift is not detected from .swift files buried in node_modules', async () => { + const dir = project({ + 'App.xcodeproj/project.pbxproj': '// pbxproj', + 'node_modules/some-dep/Vendored.swift': 'import Foundation', + }); + await expect( + SWIFT_AGENT_CONFIG.detection.detect({ installDir: dir }), + ).resolves.toBe(false); + }); +}); diff --git a/src/lib/detection/framework.ts b/src/lib/detection/framework.ts index 4efd9f6bf..74f15be28 100644 --- a/src/lib/detection/framework.ts +++ b/src/lib/detection/framework.ts @@ -19,18 +19,35 @@ export async function detectFramework( ): Promise { for (const integration of Object.values(Integration)) { const config = FRAMEWORK_REGISTRY[integration]; + // Abort the detector's filesystem walk on timeout. Without this the + // Promise.race below just stops awaiting a slow detector — the glob keeps + // walking and buffering in the background, which on huge repos climbs until + // the heap dies. Aborting stops the walk instead of leaking it. + const controller = new AbortController(); + const timeout = setTimeout(() => controller.abort(), DETECTION_TIMEOUT_MS); try { const detected = await Promise.race([ - config.detection.detect({ installDir }), - new Promise((resolve) => - setTimeout(() => resolve(false), DETECTION_TIMEOUT_MS), - ), + config.detection.detect({ installDir, signal: controller.signal }), + new Promise((resolve) => { + if (controller.signal.aborted) { + resolve(false); + return; + } + controller.signal.addEventListener('abort', () => resolve(false), { + once: true, + }); + }), ]); if (detected) { return integration; } } catch { // Skip frameworks whose detection throws + } finally { + clearTimeout(timeout); + // Stop any still-running walk before moving to the next framework + // (covers both the timeout and the detected-early paths). + controller.abort(); } } } diff --git a/src/lib/detection/glob.ts b/src/lib/detection/glob.ts new file mode 100644 index 000000000..1826aae5a --- /dev/null +++ b/src/lib/detection/glob.ts @@ -0,0 +1,83 @@ +/** + * Detection glob helpers. + * + * Framework detection runs recursive `**` globs across the whole project + * directory before any agent work starts. On large JS/TS repos those globs will + * happily descend into `node_modules` (often 100K+ entries) and `.git`, buffer + * the whole tree into memory, and OOM the process — long before the wizard ever + * gets to install PostHog. Two things keep that from happening: + * + * 1. `DETECTION_IGNORE_PATTERNS` — never walk the heavy, non-source trees. + * 2. `globWithAbort` — actually stop walking when detection times out. + */ +import fg from 'fast-glob'; +import type { Options as FastGlobOptions } from 'fast-glob'; +import type { Readable } from 'node:stream'; + +/** + * Directory trees that are never part of a project's own source and that blow + * up recursive globs on large repos. Mirrors the ignore discipline the + * Django/Flask/FastAPI detectors already follow. + */ +export const DETECTION_IGNORE_PATTERNS = [ + '**/node_modules/**', + '**/.git/**', + '**/dist/**', + '**/build/**', + '**/venv/**', + '**/.venv/**', + '**/env/**', + '**/.env/**', + '**/__pycache__/**', +]; + +/** + * Run fast-glob, but stop the underlying filesystem walk when `signal` fires. + * + * fast-glob (v3) has no `AbortSignal` support, so a plain `fg()` keeps reading + * the filesystem and buffering matches into memory even after a caller stops + * awaiting it (e.g. a `Promise.race` detection timeout). Streaming the walk lets + * us `destroy()` it on abort so memory doesn't keep climbing in the background. + * + * When no `signal` is provided this is a thin pass-through to `fg()`. + */ +export async function globWithAbort( + patterns: string | string[], + options: FastGlobOptions & { signal?: AbortSignal }, +): Promise { + const { signal, ...fgOptions } = options; + + if (!signal) { + return fg(patterns, fgOptions); + } + if (signal.aborted) { + return []; + } + + return new Promise((resolve, reject) => { + const matches: string[] = []; + // fast-glob types stream() as NodeJS.ReadableStream, but the concrete + // object is a node Readable — cast so we can destroy() the walk on abort. + const stream = fg.stream(patterns, fgOptions) as Readable; + + const onAbort = () => { + cleanup(); + stream.destroy(); + // Detection has already timed out and moved on, so this result is + // discarded — the point of resolving here is only to stop walking. + resolve(matches); + }; + const cleanup = () => signal.removeEventListener('abort', onAbort); + + signal.addEventListener('abort', onAbort, { once: true }); + stream.on('data', (entry) => matches.push(String(entry))); + stream.once('error', (err) => { + cleanup(); + reject(err); + }); + stream.once('end', () => { + cleanup(); + resolve(matches); + }); + }); +} diff --git a/src/lib/framework-config.ts b/src/lib/framework-config.ts index 5bbc7440b..2ea0aa6e6 100644 --- a/src/lib/framework-config.ts +++ b/src/lib/framework-config.ts @@ -85,6 +85,17 @@ export interface FrameworkMetadata< }; } +/** + * Options passed to a framework's `detect()` predicate. + * + * `signal` aborts any in-flight filesystem walk when detection times out, so a + * runaway glob stops reading the filesystem instead of leaking memory after the + * detection race has already moved on. + */ +export type DetectionOptions = Pick & { + signal?: AbortSignal; +}; + /** * Framework detection and version handling */ @@ -117,7 +128,7 @@ export interface FrameworkDetection { ) => Promise; /** Detect whether this framework is present in the project. */ - detect: (options: Pick) => Promise; + detect: (options: DetectionOptions) => Promise; /** Detect the project's package manager(s). Used by the in-process MCP tool. */ detectPackageManager: PackageManagerDetector;