diff --git a/.qwen/skills/memory-leak-debug/SKILL.md b/.qwen/skills/memory-leak-debug/SKILL.md new file mode 100644 index 00000000000..a9d045bece7 --- /dev/null +++ b/.qwen/skills/memory-leak-debug/SKILL.md @@ -0,0 +1,161 @@ +--- +name: memory-leak-debug +description: Diagnose memory leaks in the Qwen Code CLI using heap snapshots and + the chrome-devtools CLI. Use when investigating high memory usage, unbounded + growth, or suspected object retention issues. +--- + +# Memory Leak Debugging + +Diagnose memory leaks in the Qwen Code Node.js CLI by capturing heap snapshots +and analyzing retained object sizes via `chrome-devtools` CLI tooling. + +## Prerequisites + +- `chrome-devtools` CLI (from `chrome-devtools-mcp` package). If not found, + install with: `npm i chrome-devtools-mcp@latest -g` after user confirmation. + See https://github.com/ChromeDevTools/chrome-devtools-mcp/blob/main/docs/cli.md +- Node.js 22+ (for `--heapsnapshot-signal` support) + +## Step 1: Start the CLI with Snapshot Signal + +Use tmux so you can interact with the TUI and trigger snapshots from another +pane. Use the tmux-real-user-testing helper script: + +```bash +HELPER=.qwen/skills/tmux-real-user-testing/scripts/tmux-real-user-log.sh +eval "$(bash "$HELPER" start memleak . \ + env QWEN_CODE_NO_RELAUNCH=true NODE_OPTIONS=--heapsnapshot-signal=SIGUSR2 \ + npm run dev)" +echo "SESSION=$SESSION OUTDIR=$OUTDIR" +``` + +The `eval` exports `SESSION` and `OUTDIR`. Note: shell environment does not +persist across separate tool calls — save the session name from the output and +use it explicitly in subsequent commands. + +Notes: + +- `npm run dev` runs from TypeScript source via tsx — no build step needed and + changes to core/cli are reflected immediately. +- `QWEN_CODE_NO_RELAUNCH=true` prevents the CLI from spawning a child process, + so PID management is simpler. +- `NODE_OPTIONS` propagates the flag through npm → tsx → node. + +Get the PID of the actual node process. With `npm run dev`, there's a process +chain (npm → node scripts/dev.js → tsx → node CLI), so walk the tree to the +innermost node child: + +```bash +NODE_PID=$(bash .qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh "") +``` + +To profile the production bundle instead (e.g., verifying tree-shaking): +`npm run bundle` first, then use +`env QWEN_CODE_NO_RELAUNCH=true node --heapsnapshot-signal=SIGUSR2 dist/cli.js` +as the command. Since node is the direct pane process, PID discovery is simpler: + +```bash +NODE_PID=$(tmux list-panes -t "" -F '#{pane_pid}') +``` + +## Step 2: Exercise the Suspected Leak + +Drive the TUI via tmux (see tmux-real-user-testing skill for patterns). Take +snapshots at intervals to compare: + +```bash +kill -USR2 $NODE_PID # snapshot 1 (baseline) +# ... use the CLI via tmux send-keys ... +kill -USR2 $NODE_PID # snapshot 2 (after activity) +# ... more activity ... +kill -USR2 $NODE_PID # snapshot 3 (confirm growth trend) +``` + +Snapshots are written to the CLI's working directory as +`Heap....heapsnapshot`. + +## Step 3: Start chrome-devtools Daemon + +```bash +chrome-devtools start --experimentalMemory --headless --no-usage-statistics +``` + +This starts the daemon in file-analysis mode — no browser or live Node +connection is needed. The memory tools work entirely on `.heapsnapshot` files. + +## Step 4: Identify the Leak + +### Load and summarize + +```bash +chrome-devtools load_memory_snapshot /abs/path/to/snapshot.heapsnapshot +``` + +Returns total heap size, V8 heap breakdown, node count. + +### Get class-level aggregates with retained sizes + +```bash +chrome-devtools get_memory_snapshot_details /abs/path/to/snapshot.heapsnapshot +``` + +Output is CSV: `uid, className, count, selfSize, maxRetainedSize`. + +Compare across snapshots to find classes whose count or retained size grows +unboundedly. + +### Inspect instances of a leaking class + +```bash +chrome-devtools get_nodes_by_class /abs/path/to/snapshot.heapsnapshot +``` + +Where `` is from the `get_memory_snapshot_details` output. Returns +individual instances with their `id`, `retainedSize`, and `nodeIndex`. + +### Trace retainer chains + +```bash +chrome-devtools get_node_retainers /abs/path/to/snapshot.heapsnapshot +``` + +Where `` is the `id` field from `get_nodes_by_class`. Shows what holds +the object alive — follow the chain to find the root retention path. + +## Step 5: Identify Root Cause + +Common patterns: + +- **Unbounded buffer/array**: An array that accumulates entries without eviction + (e.g., `performance.measure()` → `measureEntryBuffer`). +- **Event listener leak**: Listeners registered on long-lived emitters without + cleanup. +- **Closure capture**: A closure inadvertently captures a large object that + outlives its intended scope. +- **Module-level cache**: A Map/Set at module scope that grows with usage. + +The retainer chain tells you _what_ holds the object; the class aggregate +growth rate tells you _how fast_ it leaks. + +## Step 6: Verify Fix + +After applying the fix: + +1. Rebuild: `npm run bundle` +2. Repeat Steps 1-4 with the same workload. +3. Confirm the leaking class count stabilizes (no longer grows with activity). + +## Cleanup + +```bash +HELPER=.qwen/skills/tmux-real-user-testing/scripts/tmux-real-user-log.sh +bash "$HELPER" finish "" "" +chrome-devtools stop +rm *.heapsnapshot # if no longer needed +``` + +## Worked Example + +See `examples/react-reconciler-performance-measure-leak.md` for the ink 7 +upgrade leak that caused ~143 MB retention from `PerformanceMeasure` objects. diff --git a/.qwen/skills/memory-leak-debug/examples/react-reconciler-performance-measure-leak.md b/.qwen/skills/memory-leak-debug/examples/react-reconciler-performance-measure-leak.md new file mode 100644 index 00000000000..f5db329af57 --- /dev/null +++ b/.qwen/skills/memory-leak-debug/examples/react-reconciler-performance-measure-leak.md @@ -0,0 +1,65 @@ +# React Reconciler PerformanceMeasure Leak + +## Symptom + +After the ink 6→7 upgrade (v0.15.11), moderate CLI usage caused heap to grow +to 300+ MB. RSS climbed steadily and never stabilized. + +## Diagnosis + +### Snapshot comparison + +Took 5 snapshots over ~25 minutes of normal usage. + +Snapshot #1 (baseline): + +``` +PerformanceMeasure: count=184, retainedSize=184 kB +``` + +Snapshot #5 (after activity): + +``` +PerformanceMeasure: count=150,716, retainedSize=146,798 kB (~143 MB) +``` + +Growth: ~800x over the session. Linear with number of React renders. + +### Retainer chain + +``` +chrome-devtools get_node_retainers 1003471 +``` + +Showed `PerformanceMeasure` instances retained by `(object elements)` → `Array` +— the global `measureEntryBuffer` that Node.js maintains for +`performance.measure()` calls. + +### Source identification + +`react-reconciler` ≥0.33 (pulled in by ink 7) calls `performance.measure()` on +every component render in its **development build**. The dev/prod build is +selected at runtime via `process.env.NODE_ENV`. Since the esbuild config never +set `NODE_ENV` to `"production"`, the bundle shipped both builds and selected +dev at runtime. + +## Fix + +Set `process.env.NODE_ENV` to `"production"` in esbuild's `define` map so the +conditional require resolves statically and the entire 15K-line dev build is +tree-shaken: + +```js +// esbuild.config.js +define: { + 'process.env.NODE_ENV': JSON.stringify('production'), +} +``` + +Bundle shrank by ~700 KB / 15,800 lines. PerformanceMeasure objects no longer +accumulate. + +## Commit + +`dbdc94be9` — fix(build): tree-shake React reconciler dev build to prevent +PerformanceMeasure leak diff --git a/.qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh b/.qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh new file mode 100755 index 00000000000..7a5ffd77c0a --- /dev/null +++ b/.qwen/skills/memory-leak-debug/scripts/find-leaf-node.sh @@ -0,0 +1,16 @@ +#!/usr/bin/env bash +# Find the innermost node child process in a tmux session. +# Usage: find-leaf-node.sh +set -euo pipefail + +session=${1:?Usage: find-leaf-node.sh } + +pid=$(tmux list-panes -t "$session" -F '#{pane_pid}' | head -1) + +while true; do + child=$(pgrep -P "$pid" node 2>/dev/null | head -1 || true) + [ -z "$child" ] && break + pid=$child +done + +echo "$pid" diff --git a/packages/core/src/extension/redaction.test.ts b/packages/core/src/extension/redaction.test.ts new file mode 100644 index 00000000000..137217c142a --- /dev/null +++ b/packages/core/src/extension/redaction.test.ts @@ -0,0 +1,77 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { REDACTED_URL_CREDENTIAL, redactUrlCredentials } from './redaction.js'; + +describe('redactUrlCredentials', () => { + it('redacts username and password from HTTPS URLs', () => { + expect( + redactUrlCredentials('https://user:token@example.com/org/repo.git'), + ).toBe(`https://${REDACTED_URL_CREDENTIAL}@example.com/org/repo.git`); + }); + + it('redacts token-only URL credentials', () => { + expect( + redactUrlCredentials('https://ghp_token@github.com/owner/repo'), + ).toBe(`https://${REDACTED_URL_CREDENTIAL}@github.com/owner/repo`); + }); + + it('redacts raw hash characters echoed in URL credentials', () => { + expect( + redactUrlCredentials('https://user:pass#word@example.com/org/repo.git'), + ).toBe(`https://${REDACTED_URL_CREDENTIAL}@example.com/org/repo.git`); + }); + + it('redacts unencoded at signs inside echoed URL credentials', () => { + expect( + redactUrlCredentials('https://email@gmail.com:tok@example.com/repo'), + ).toBe(`https://${REDACTED_URL_CREDENTIAL}@example.com/repo`); + }); + + it('redacts unencoded question marks inside echoed URL credentials', () => { + expect(redactUrlCredentials('https://user:gh?token@example.com/repo')).toBe( + `https://${REDACTED_URL_CREDENTIAL}@example.com/repo`, + ); + }); + + it('redacts percent-encoded URL credentials', () => { + expect( + redactUrlCredentials('https://user%40mail:tok%3Fen@example.com/repo'), + ).toBe(`https://${REDACTED_URL_CREDENTIAL}@example.com/repo`); + }); + + it('does not redact at signs after the URL path starts', () => { + const source = 'https://example.com/path/@scope/package'; + expect(redactUrlCredentials(source)).toBe(source); + }); + + it('redacts custom URL schemes used by extension sources', () => { + expect(redactUrlCredentials('sso://user:token@example.com/org/repo')).toBe( + `sso://${REDACTED_URL_CREDENTIAL}@example.com/org/repo`, + ); + }); + + it('redacts credentialed URLs embedded in diagnostic messages', () => { + expect( + redactUrlCredentials( + 'fatal: authentication failed for https://user:token@example.com/repo', + ), + ).toBe( + `fatal: authentication failed for https://${REDACTED_URL_CREDENTIAL}@example.com/repo`, + ); + }); + + it('does not modify URLs without credentials', () => { + const source = 'https://github.com/owner/repo'; + expect(redactUrlCredentials(source)).toBe(source); + }); + + it('does not throw for malformed or non-URL sources', () => { + expect(redactUrlCredentials('owner/repo')).toBe('owner/repo'); + expect(redactUrlCredentials('https://')).toBe('https://'); + }); +}); diff --git a/packages/core/src/extension/redaction.ts b/packages/core/src/extension/redaction.ts new file mode 100644 index 00000000000..3c1d27a1523 --- /dev/null +++ b/packages/core/src/extension/redaction.ts @@ -0,0 +1,22 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +export const REDACTED_URL_CREDENTIAL = '***REDACTED***'; + +const URL_CREDENTIALS_PATTERN = /\b([a-z][a-z0-9+.-]*:\/\/)(?:[^/\s]+@)+/gi; + +/** + * Redacts userinfo credentials from URL-like extension sources for logs, + * telemetry, and display. This also handles diagnostic messages that contain + * credentialed URLs. The original source should still be preserved for + * installation and update operations. + */ +export function redactUrlCredentials(source: string): string { + return source.replace( + URL_CREDENTIALS_PATTERN, + `$1${REDACTED_URL_CREDENTIAL}@`, + ); +} diff --git a/packages/core/src/utils/projectRoot.test.ts b/packages/core/src/utils/projectRoot.test.ts new file mode 100644 index 00000000000..d29295a7181 --- /dev/null +++ b/packages/core/src/utils/projectRoot.test.ts @@ -0,0 +1,86 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, it, expect, beforeEach, afterEach } from 'vitest'; +import * as fsPromises from 'node:fs/promises'; +import * as os from 'node:os'; +import * as path from 'node:path'; +import { findProjectRoot } from './projectRoot.js'; + +describe('findProjectRoot', () => { + let testRootDir: string; + let projectRoot: string; + let subDir: string; + + beforeEach(async () => { + testRootDir = await fsPromises.mkdtemp( + path.join(os.tmpdir(), 'find-project-root-'), + ); + projectRoot = path.join(testRootDir, 'project'); + subDir = path.join(projectRoot, 'src', 'nested'); + await fsPromises.mkdir(subDir, { recursive: true }); + }); + + afterEach(async () => { + await fsPromises.rm(testRootDir, { + recursive: true, + force: true, + maxRetries: 3, + retryDelay: 10, + }); + }); + + it('returns the project root when .git is a directory (normal clone)', async () => { + await fsPromises.mkdir(path.join(projectRoot, '.git')); + + expect(await findProjectRoot(subDir)).toBe(projectRoot); + expect(await findProjectRoot(projectRoot)).toBe(projectRoot); + }); + + it('returns the project root when .git is a FILE (git worktree / submodule layout)', async () => { + // Git worktrees and submodules mark the repo root with a `.git` file + // containing `gitdir: `. The old implementation only checked + // `stats.isDirectory()` and silently returned null here — the bug + // that prompted the extraction. + await fsPromises.writeFile( + path.join(projectRoot, '.git'), + 'gitdir: /elsewhere/worktrees/feature/.git\n', + ); + + expect(await findProjectRoot(subDir)).toBe(projectRoot); + expect(await findProjectRoot(projectRoot)).toBe(projectRoot); + }); + + it('returns null when no .git ancestor exists', async () => { + // No .git anywhere — neither directory nor file. + expect(await findProjectRoot(subDir)).toBeNull(); + }); + + it('walks up past intermediate directories without .git', async () => { + // Only the outermost has .git; intermediates do not. + await fsPromises.mkdir(path.join(projectRoot, '.git')); + const deep = path.join(projectRoot, 'a', 'b', 'c', 'd'); + await fsPromises.mkdir(deep, { recursive: true }); + + expect(await findProjectRoot(deep)).toBe(projectRoot); + }); + + it('treats a .git symlink to a directory as a project root', async () => { + // Edge: some setups symlink .git. lstat would NOT follow the link, + // so this pins the behavior we get with the directory-or-file shape: + // a symlink to a directory should still be recognized via the file + // branch (lstat reports it as a symlink, which is neither — so this + // documents the current behavior, not a guarantee). + const target = path.join(testRootDir, 'real-git'); + await fsPromises.mkdir(target); + await fsPromises.symlink(target, path.join(projectRoot, '.git')); + + // Symlinks aren't directories or regular files under lstat. Document + // that we do NOT chase them — caller would see null and fall back. + // If this assertion ever needs to flip, do it deliberately. + expect(await findProjectRoot(projectRoot)).toBeNull(); + }); +}); diff --git a/packages/core/src/utils/projectRoot.ts b/packages/core/src/utils/projectRoot.ts new file mode 100644 index 00000000000..4d84a9c2a83 --- /dev/null +++ b/packages/core/src/utils/projectRoot.ts @@ -0,0 +1,74 @@ +/** + * @license + * Copyright 2025 Google LLC + * SPDX-License-Identifier: Apache-2.0 + */ + +import * as fs from 'node:fs/promises'; +import * as path from 'node:path'; +import { createDebugLogger } from './debugLogger.js'; + +const logger = createDebugLogger('PROJECT_ROOT'); + +/** + * Walk up from `startDir` looking for the nearest ancestor that contains a + * `.git` entry, and return that ancestor's path. Returns `null` if no + * ancestor up to the filesystem root has `.git`. + * + * `.git` is a directory in a normal clone but a regular file (containing + * `gitdir: `) in git worktrees and submodules. Both shapes mark a + * repo root — this helper accepts either, so callers don't silently break + * for worktree / submodule users. + * + * Symlinks are intentionally not chased: `lstat` reports them as + * `isSymbolicLink()`, which is neither a directory nor a regular file, so + * the walk continues past them. That preserves the behavior the previous + * private copies in `memoryDiscovery.ts` and `memoryImportProcessor.ts` + * had. + */ +export async function findProjectRoot( + startDir: string, +): Promise { + let currentDir = path.resolve(startDir); + while (true) { + const gitPath = path.join(currentDir, '.git'); + try { + const stats = await fs.lstat(gitPath); + if (stats.isDirectory() || stats.isFile()) { + return currentDir; + } + } catch (error: unknown) { + // ENOENT is the expected case while walking up — don't log it. + // Tests often mock fs in ways that throw non-ENOENT errors; stay + // quiet there too. + const isENOENT = + typeof error === 'object' && + error !== null && + 'code' in error && + (error as { code: string }).code === 'ENOENT'; + + const isTestEnv = + process.env['NODE_ENV'] === 'test' || process.env['VITEST']; + + if (!isENOENT && !isTestEnv) { + if (typeof error === 'object' && error !== null && 'code' in error) { + const fsError = error as { code: string; message: string }; + logger.warn( + `Error checking for .git at ${gitPath}: ${fsError.message}`, + ); + } else { + logger.warn( + `Non-standard error checking for .git at ${gitPath}: ${String( + error, + )}`, + ); + } + } + } + const parentDir = path.dirname(currentDir); + if (parentDir === currentDir) { + return null; + } + currentDir = parentDir; + } +}