From d581da04ddcfe5fc41f874471a178a371d91afb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E7=A7=A6=E5=A5=87?= Date: Wed, 13 May 2026 10:20:28 +0800 Subject: [PATCH] fix(cli): resolve chunk-relative sibling paths under esbuild splitting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit With `splitting: true`, esbuild hoists modules with shared dependencies into `dist/chunks/`. Three modules derived runtime paths from `import.meta.url` assuming they were co-located with `cli.js`; once hoisted, `path.dirname(fileURLToPath(import.meta.url))` resolved to `dist/chunks/` and sibling-asset lookups silently missed: - `skill-manager.ts`: bundledSkillsDir → `dist/chunks/bundled` (actual `dist/bundled/`). The `existsSync` guard swallowed the miss, dropping all four bundled skills (`/review`, `/qc-helper`, `/batch`, `/loop`) with no user-visible signal. - `ripgrepUtils.ts`: `getBuiltinRipgrep()` → `dist/chunks/vendor/...`. Falls back to system rg if installed, otherwise null on minimal hosts — degrading grep to the slow internal scanner. - `i18n/index.ts`: `getBuiltinLocalesDir()` → `dist/chunks/locales`. User-visible behavior survives via the static glob import in `tryImportBundledTranslations`, but the loose-on-disk override path is dead. Each module now strips a trailing `chunks` segment when present, so the lookup resolves under `dist/`. In source / transpiled modes the basename is never `chunks`, so the fallback is a no-op. Also: - Add `chunks` to `DIST_REQUIRED_PATHS` in `create-standalone-package.js` so a regressed bundle that produces only `cli.js` fails the pre-packaging check instead of shipping a broken archive. - Expand `esbuild-shims.js` header so future contributors understand that `__qwen_filename` / `__qwen_dirname` always resolve to the shim's chunk file (dist/chunks/) and that sibling-asset lookups must strip the `chunks` segment. Reported by claude-opus-4-7 via Qwen Code /qreview on #4070. Generated with AI Co-authored-by: Qwen-Coder --- packages/cli/src/i18n/index.ts | 11 ++++++++++- packages/core/src/skills/skill-manager.ts | 15 +++++++++++---- packages/core/src/utils/ripgrepUtils.ts | 13 +++++++++++-- scripts/create-standalone-package.js | 7 ++++++- scripts/esbuild-shims.js | 11 +++++++++++ 5 files changed, 49 insertions(+), 8 deletions(-) diff --git a/packages/cli/src/i18n/index.ts b/packages/cli/src/i18n/index.ts index dd4028750..052df05f0 100644 --- a/packages/cli/src/i18n/index.ts +++ b/packages/cli/src/i18n/index.ts @@ -44,7 +44,16 @@ type TranslationLoadResult = // Path helpers const getBuiltinLocalesDir = (): string => { const __filename = fileURLToPath(import.meta.url); - return path.join(path.dirname(__filename), 'locales'); + // When bundled with esbuild code-splitting, this module is hoisted into a + // shared chunk under `dist/chunks/`. The locales directory is still copied + // to `dist/locales/` (a sibling of cli.js), so strip the trailing `chunks` + // segment so the lookup resolves under `dist/`. In source / transpiled + // modes the basename is never `chunks`, so this is a no-op. + let moduleDir = path.dirname(__filename); + if (path.basename(moduleDir) === 'chunks') { + moduleDir = path.dirname(moduleDir); + } + return path.join(moduleDir, 'locales'); }; const getUserLocalesDir = (): string => diff --git a/packages/core/src/skills/skill-manager.ts b/packages/core/src/skills/skill-manager.ts index 0c7c62dbf..149037346 100644 --- a/packages/core/src/skills/skill-manager.ts +++ b/packages/core/src/skills/skill-manager.ts @@ -86,10 +86,17 @@ export class SkillManager { private activationRegistry: SkillActivationRegistry | null = null; constructor(private readonly config: Config) { - this.bundledSkillsDir = path.join( - path.dirname(fileURLToPath(import.meta.url)), - 'bundled', - ); + // When bundled with esbuild code-splitting, this module is hoisted into + // a shared chunk under `dist/chunks/`. The bundled skills directory is + // still copied to `dist/bundled/` by `copy_bundle_assets.js`, so strip + // the trailing `chunks` segment so the sibling lookup resolves under + // `dist/` rather than `dist/chunks/`. In source / transpiled modes the + // basename is never `chunks`, so this is a no-op. + let moduleDir = path.dirname(fileURLToPath(import.meta.url)); + if (path.basename(moduleDir) === 'chunks') { + moduleDir = path.dirname(moduleDir); + } + this.bundledSkillsDir = path.join(moduleDir, 'bundled'); } /** diff --git a/packages/core/src/utils/ripgrepUtils.ts b/packages/core/src/utils/ripgrepUtils.ts index 7f6f3d9c8..bfd23bccd 100644 --- a/packages/core/src/utils/ripgrepUtils.ts +++ b/packages/core/src/utils/ripgrepUtils.ts @@ -57,9 +57,18 @@ function wslTimeout(): number { : RIPGREP_RUN_TIMEOUT_MS; } -// Get the directory of the current module +// Get the directory of the current module. When bundled with esbuild +// code-splitting, this file is hoisted into `dist/chunks/`, but the vendored +// ripgrep binary is still copied to `dist/vendor/` (a sibling of cli.js). +// Strip the trailing `chunks` segment so the lookup below resolves under +// `dist/`. In source / transpiled modes the basename is never `chunks`, so +// the fallback is a no-op. const __filename = fileURLToPath(import.meta.url); -const __dirname = path.dirname(__filename); +const __dirnameRaw = path.dirname(__filename); +const __dirname = + path.basename(__dirnameRaw) === 'chunks' + ? path.dirname(__dirnameRaw) + : __dirnameRaw; type Platform = 'darwin' | 'linux' | 'win32'; type Architecture = 'x64' | 'arm64'; diff --git a/scripts/create-standalone-package.js b/scripts/create-standalone-package.js index 3ed860533..4f6e52f33 100644 --- a/scripts/create-standalone-package.js +++ b/scripts/create-standalone-package.js @@ -36,7 +36,12 @@ const TARGETS = new Map([ ['win-x64', { outputExtension: 'zip', nodeExecutable: ['node.exe'] }], ]); -const DIST_REQUIRED_PATHS = ['cli.js', 'vendor', 'bundled/qc-helper/docs']; +const DIST_REQUIRED_PATHS = [ + 'cli.js', + 'chunks', + 'vendor', + 'bundled/qc-helper/docs', +]; const DIST_ALLOWED_ENTRIES = new Set([ 'cli.js', 'chunks', diff --git a/scripts/esbuild-shims.js b/scripts/esbuild-shims.js index fbdfa6776..2a3427641 100644 --- a/scripts/esbuild-shims.js +++ b/scripts/esbuild-shims.js @@ -27,5 +27,16 @@ if (typeof globalThis.require === 'undefined') { } export const require = _require; +// IMPORTANT: __qwen_filename / __qwen_dirname always resolve to this shim's +// chunk file — i.e. dist/chunks/ in a built bundle, NOT the directory of any +// source file that uses bare __dirname / __filename. esbuild's `define` +// rewrites all free references in source code to these symbols, so to get a +// per-file path you MUST declare a local shadow at the top of your module: +// const __filename = fileURLToPath(import.meta.url); +// const __dirname = path.dirname(__filename); +// Even with a local shadow, under code-splitting the path can still point to +// dist/chunks/ rather than the source dir — sibling-asset lookups (vendor/, +// bundled/, locales/) must strip a trailing `chunks` segment. See +// skill-manager.ts / ripgrepUtils.ts / i18n/index.ts for the pattern. export const __qwen_filename = fileURLToPath(import.meta.url); export const __qwen_dirname = dirname(__qwen_filename);