From 065e7c3de003c817439a30f2dae9a5eb35898b21 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 25 Jul 2026 00:12:08 +0800 Subject: [PATCH 1/5] perf(core): Lazy-load first-use dependencies Co-authored-by: Qwen-Coder --- docs/design/lazy-first-use-dependencies.md | 160 +++++++++++++++++ esbuild.config.js | 27 +++ packages/cli/src/cli.ts | 2 +- packages/cli/src/serve/core-runtime.ts | 34 ++++ .../cli/src/serve/fs/workspace-file-system.ts | 8 +- packages/cli/src/serve/run-qwen-serve.ts | 4 +- .../src/serve/workspace-registration-store.ts | 4 +- packages/cli/src/ui/utils/export/collect.ts | 4 +- packages/cli/src/utils/cleanup.ts | 2 +- .../cli/src/utils/deferred-core-runtime.ts | 12 ++ packages/core/src/config/config.ts | 5 +- packages/core/src/extension/github.ts | 6 +- packages/core/src/index.ts | 5 +- .../src/services/fileSystemService.test.ts | 2 +- .../core/src/services/fileSystemService.ts | 64 +++++-- .../core/src/services/gitWorktreeService.ts | 169 ++++++++++-------- .../src/services/shellExecutionService.ts | 26 ++- packages/core/src/services/worktreeCleanup.ts | 3 +- packages/core/src/utils/encoding.ts | 10 ++ packages/core/src/utils/fileUtils.test.ts | 2 +- packages/core/src/utils/fileUtils.ts | 33 ++-- packages/core/src/utils/iconvHelper.ts | 6 +- .../core/src/utils/load-iconv-lite.test.ts | 30 ++++ packages/core/src/utils/load-iconv-lite.ts | 20 +++ .../core/src/utils/load-simple-git.test.ts | 58 ++++++ packages/core/src/utils/load-simple-git.ts | 34 ++++ .../src/utils/load-xterm-headless.test.ts | 34 ++++ .../core/src/utils/load-xterm-headless.ts | 24 +++ packages/core/src/utils/read-text-range.ts | 2 +- packages/core/src/utils/sync-file-encoding.ts | 82 +++++++++ scripts/check-serve-fast-path-bundle.js | 5 + .../serve-fast-path-bundle-check.test.js | 42 +++++ 32 files changed, 787 insertions(+), 132 deletions(-) create mode 100644 docs/design/lazy-first-use-dependencies.md create mode 100644 packages/cli/src/serve/core-runtime.ts create mode 100644 packages/cli/src/utils/deferred-core-runtime.ts create mode 100644 packages/core/src/utils/encoding.ts create mode 100644 packages/core/src/utils/load-iconv-lite.test.ts create mode 100644 packages/core/src/utils/load-iconv-lite.ts create mode 100644 packages/core/src/utils/load-simple-git.test.ts create mode 100644 packages/core/src/utils/load-simple-git.ts create mode 100644 packages/core/src/utils/load-xterm-headless.test.ts create mode 100644 packages/core/src/utils/load-xterm-headless.ts create mode 100644 packages/core/src/utils/sync-file-encoding.ts diff --git a/docs/design/lazy-first-use-dependencies.md b/docs/design/lazy-first-use-dependencies.md new file mode 100644 index 00000000000..8b20ec0a071 --- /dev/null +++ b/docs/design/lazy-first-use-dependencies.md @@ -0,0 +1,160 @@ +# Lazy First-Use Loading for Encoding, Terminal, and Git Dependencies + +## Context + +Issue #7264 tracks dependencies that are present in the ACP child process's eager static import closure even though most sessions never use them. Candidate 5 groups three packages with distinct first-use boundaries: + +| Package | Baseline ACP closure | First use | +| ------------------------ | -------------------: | ----------------------------------------------------------------- | +| `iconv-lite` | 551,713 bytes | Reading or writing non-UTF-8 text without a BOM | +| `@xterm/headless` | 213,071 bytes | Starting a shell through the PTY path | +| `simple-git` | 146,526 bytes | Performing a worktree, cleanup, or GitHub extension Git operation | +| **Direct package total** | **911,310 bytes** | | + +The direct total is approximately 890 KiB. The complete ACP static closure also contains modules that become unreachable when these packages move off the eager path, so the measured bundle-level reduction can be larger. + +## Goals + +- Remove all three packages from the ACP child's static import closure. +- Preserve the current synchronous public encoding helpers. +- Load each package once, at its first real use, with no new configuration. +- Preserve shell fallback behavior, Git behavior, file encoding metadata, BOM handling, and atomic writes. +- Add a bundle guard so future imports cannot silently restore these packages to the eager closure. +- Validate the change using the same 2-vCPU, 4-GiB acceptance discipline as the other candidates in #7264. + +## Non-goals + +- Changing the public encoding APIs from synchronous to asynchronous. +- Replacing `iconv-lite`, `@xterm/headless`, or `simple-git`. +- Changing PTY selection, worktree semantics, encoding detection, or error policy. +- Optimizing code that runs after these dependencies have already been loaded. + +## Import-Closure Findings + +The baseline bundle built from `febb43bc9266cc7a3363539df87d90d752ad782c` has an ACP static closure of 13,405,027 bytes across 144 outputs. An esbuild metafile traversal attributes 551,713 bytes to `iconv-lite`, 213,071 bytes to `@xterm/headless`, and 146,526 bytes to `simple-git`. + +The initial package-level lazy imports were not sufficient. The CLI contained production namespace dynamic imports of the Core package root. In an esbuild code-splitting build, requesting the entire namespace keeps every root export reachable, including the synchronous encoding compatibility export. The design therefore requires both dependency-local loaders and narrow CLI runtime entry modules that re-export only the symbols each deferred path consumes. + +## Design + +### Shared loader properties + +Each package has a package-local loader backed by a module-scoped promise. Concurrent first users share the same import, and later users reuse the resolved module. The loaders normalize the CommonJS interop shapes emitted by Node and esbuild and expose only the runtime members their consumers need. + +The loaders deliberately use `import()` rather than `createRequire()`. The production bundle is standalone and must not depend on a separately installed `node_modules` tree. Dynamic imports let esbuild emit self-contained chunks while keeping those chunks outside the ACP static closure. + +### `@xterm/headless` + +`ShellExecutionService.execute()` is already asynchronous. The service first obtains the PTY implementation, then loads `@xterm/headless` immediately before entering the PTY execution path. It rechecks the abort signal after the asynchronous import and passes the resolved `Terminal` constructor into the existing synchronous PTY and replay helpers. + +If the terminal chunk fails to load, the error remains inside the existing PTY failure boundary and execution falls back to `child_process`, matching the current fallback policy. No package load occurs when PTY support is unavailable or the child-process path is selected. + +### `simple-git` + +All real Git operations in the audited consumers are asynchronous. `GitWorktreeService` keeps construction side-effect-free and resolves a per-instance `SimpleGit` promise only when its first Git method is called. Other Core consumers use the same package-local loader directly. + +Startup cleanup first uses the existing lightweight repository-root discovery. It loads `simple-git` only when a real repository is present and stale worktree inspection is necessary. A failed import rejects the operation at the same asynchronous boundary where a Git initialization failure was already reported. + +### `iconv-lite` + +This package has the main compatibility constraint: `decodeBufferWithEncodingInfo()` and `encodeTextFileContent()` are public synchronous APIs. JavaScript dynamic import is asynchronous, so making these functions directly lazy would be an API break. + +The synchronous APIs remain available through a compatibility module that statically imports `iconv-lite`. Only the Core root re-export edge is marked side-effect-free for the bundle, allowing esbuild to discard the compatibility module when a particular entry does not use those exports. Other imports of the module retain normal side-effect treatment. + +Internal asynchronous file-service paths use lazy variants: + +- Empty, BOM-tagged, valid UTF-8, ASCII, and UTF-8 writes complete without loading `iconv-lite`. +- A detected non-UTF-8 read loads the codec before decoding. +- A write that preserves non-UTF-8 metadata loads the codec before encoding. +- A read-side load or decode failure retains the current warning and UTF-8 replacement fallback. +- A write-side load or encode failure rejects the write instead of corrupting bytes. + +The CLI's deferred Core namespace imports are replaced by narrow local runtime entry modules. This avoids retaining every Core root export while preserving the same bundled Core instance and class identity. + +## Bundle Guard + +The ACP fast-path guard treats `iconv-lite`, `@xterm/headless`, and `simple-git` as forbidden static packages. A static path from the ACP entry fails the check; dynamic-only paths are allowed. Tests cover both rejection and allowed dynamic boundaries. + +This guard evaluates the metafile import graph rather than bundle text, so a renamed chunk or minified symbol cannot bypass it. + +## Compatibility and Failure Audit + +| Area | Preserved behavior | New boundary | +| ------------------------------- | -------------------------------------------------------------- | ----------------------------------------------------------------------- | +| Shell execution | PTY output handling, replay, abort, child-process fallback | Terminal chunk is loaded after PTY selection | +| Worktrees and GitHub extensions | Existing `simple-git` options and error propagation | Git module is loaded on the first asynchronous Git operation | +| Text reads | BOM and UTF-8 fast paths, encoding metadata, fallback decoding | Codec is loaded only for a detected non-UTF-8 fallback | +| Text writes | BOM preservation, non-UTF-8 encoding, atomic write behavior | Codec is loaded only when non-UTF-8 metadata requires it | +| Public Core API | Synchronous encoding helper signatures and behavior | Compatibility export can be tree-shaken from entries that do not use it | + +The design does not introduce process-global mutable configuration. Loader promises are process-local and idempotent. Rejected imports remain rejected, which is appropriate because a missing or corrupt bundled chunk cannot recover during the same process lifetime. + +## Alternatives Considered + +### Convert the synchronous encoding APIs to promises + +Rejected because it breaks public callers and widens an otherwise internal startup optimization. + +### Use `createRequire()` at first use + +Rejected because it would make the bundled CLI depend on a runtime `node_modules` installation and would not produce a self-contained release artifact. + +### Reimplement the encoding tables or terminal behavior + +Rejected as substantially riskier than deferring the existing packages. + +### Land only `@xterm/headless` and `simple-git` + +This would be simpler, but it would leave the largest package in the group on the eager path and would not satisfy candidate 5. The compatibility facade and narrow runtime entry modules remove `iconv-lite` without changing its public API. + +## Verification Plan + +1. Build the CLI-only production artifacts and bundle them with esbuild code splitting. +2. Traverse the ACP entry's static metafile closure and require zero attributed bytes for all three packages. +3. Run focused unit tests for encoding reads and writes, shell execution and fallback, Git worktree behavior, cleanup, GitHub extension operations, each loader, and the bundle guard. +4. Run the affected CLI tests, build, and full typecheck. +5. On the 2-vCPU, 4-GiB reference host, run one paired smoke test followed by 30 alternating serial cold pairs and 30 preheated pairs. Report `channel.initialize`, process-to-first-session latency, peak process-tree RSS, concurrency, telemetry-disabled behavior, legacy single-session behavior, and residual processes. + +## Measured Static Result + +| Variant | ACP outputs | ACP static closure | `iconv-lite` | `@xterm/headless` | `simple-git` | +| --------- | ----------: | -------------------: | -------------: | ----------------: | -------------: | +| Baseline | 144 | 13,405,027 bytes | 551,713 bytes | 213,071 bytes | 146,526 bytes | +| Candidate | 142 | 12,314,617 bytes | 0 bytes | 0 bytes | 0 bytes | +| Delta | −2 | **−1,090,410 bytes** | −551,713 bytes | −213,071 bytes | −146,526 bytes | + +The remote performance result must be evaluated separately because bundle bytes do not imply a latency improvement. + +## Measured 2C4G Result + +The remote host had 2 vCPUs, 3.5 GiB total RAM, no swap, and Node.js 22.23.1. A separate one-pair smoke run and its functional scenarios passed before the formal run. The formal run then completed 30 alternating serial cold pairs and 30 alternating preheated pairs, followed by another set of functional scenarios, with no failed sessions or residual processes. + +The formal candidate was the copied prototype artifact with SHA-256 `f0ac7edc7665752efac7b7bfbb4fb055ce2d8ef1a8ae5dd1af630305a2c84d28`, labeled `febb43bc9266cc7a3363539df87d90d752ad782c+candidate5` by the harness. The result applies to that exact artifact, not to a future commit SHA; a PR should retain the artifact hash or rerun the gate if its production code changes. + +| Scenario | Metric | Baseline P50 / P95 | Candidate P50 / P95 | P50 delta | Paired median | Candidate wins | +| --------- | ----------------------- | -----------------: | ------------------: | ------------: | ------------: | -------------: | +| Cold | `channel.initialize` | 896.2 / 915.5 ms | 831.5 / 848.5 ms | **−64.7 ms** | −60.1 ms | 30/30 | +| Cold | `POST /session` | 1273.8 / 1305.3 ms | 1156.5 / 1181.1 ms | **−117.4 ms** | −105.1 ms | 30/30 | +| Cold | process → first session | 1877.7 / 1921.0 ms | 1733.3 / 1763.8 ms | **−144.4 ms** | −136.2 ms | 30/30 | +| Cold | peak process-tree RSS | 417.0 / 451.4 MB | 408.1 / 419.2 MB | **−8.9 MB** | −8.5 MB | 18/30 | +| Preheated | `channel.initialize` | 895.3 / 926.3 ms | 837.2 / 861.6 ms | **−58.1 ms** | −49.2 ms | 30/30 | +| Preheated | `POST /session` | 90.0 / 94.2 ms | 83.3 / 86.7 ms | **−6.7 ms** | −6.5 ms | 28/30 | +| Preheated | process → first session | 3697.3 / 3723.0 ms | 3666.0 / 3676.6 ms | **−31.3 ms** | −29.6 ms | 30/30 | +| Preheated | peak process-tree RSS | 430.5 / 433.1 MB | 403.0 / 419.3 MB | **−27.5 MB** | −13.9 MB | 19/30 | + +The candidate also passed concurrent first sessions, telemetry-disabled startup, and legacy single-session startup. A production-configured first-use probe passed GBK encode/decode, headless terminal construction and write, loader single-flight identity, and a real local `simple-git` repository initialization. The remote host has no `git` executable, so the remote `simple-git` probe verified module loading and factory construction but could not execute a real Git command; the full local Git service suites cover those operations. + +The acceptance gate is satisfied: the cold-path wins are consistent across all 30 latency pairs, remain visible in the preheated channel initialization metric, and do not trade latency for higher memory. + +## Risks and Rollout + +The main risk is a first-use-only failure that eager imports previously exposed at startup. Focused tests exercise the first-use paths, and the production bundle guard verifies that the imports remain dynamic. Remote smoke and acceptance runs exercise real bundled ACP sessions and check for residual processes. + +This candidate should remain a separate PR, as required by #7264, so its regression surface and performance effect stay attributable. If the 2C4G gate shows no repeatable startup benefit or a meaningful first-use regression, the implementation should not land solely for bundle-size reduction. + +## References + +- [esbuild code splitting](https://esbuild.github.io/api/#splitting) +- [esbuild metafile analysis](https://esbuild.github.io/api/#metafile) +- [Node.js dynamic import expressions](https://nodejs.org/api/esm.html#import-expressions) +- [Node.js CommonJS interoperability](https://nodejs.org/api/esm.html#interoperability-with-commonjs) diff --git a/esbuild.config.js b/esbuild.config.js index 1564e187bd7..613b8d462af 100644 --- a/esbuild.config.js +++ b/esbuild.config.js @@ -116,6 +116,32 @@ const sdkNodeExporterStubPlugin = { }, }; +const syncFileEncodingTreeShakePlugin = { + name: 'sync-file-encoding-tree-shake', + setup(build) { + build.onResolve( + { filter: /^\.\/utils\/sync-file-encoding\.js$/ }, + (args) => { + if ( + !/[\\/]packages[\\/]core[\\/](?:src|dist)[\\/]index\.(?:ts|js)$/.test( + args.importer, + ) + ) { + return null; + } + const sourceExtension = args.importer.endsWith('.ts') ? '.ts' : '.js'; + return { + path: path.resolve( + args.resolveDir, + args.path.replace(/\.js$/, sourceExtension), + ), + sideEffects: false, + }; + }, + ); + }, +}; + const external = [ '@lydell/node-pty', 'node-pty', @@ -204,6 +230,7 @@ const mainBuild = esbuild.build({ loader: { '.node': 'file' }, plugins: [ sdkNodeExporterStubPlugin, + syncFileEncodingTreeShakePlugin, wasmBinaryPlugin, wasmLoader({ mode: 'embedded' }), ], diff --git a/packages/cli/src/cli.ts b/packages/cli/src/cli.ts index e9743bbf0ea..b4d95c662fd 100644 --- a/packages/cli/src/cli.ts +++ b/packages/cli/src/cli.ts @@ -421,7 +421,7 @@ export function isExpectedPtyRaceError(error: unknown): boolean { export async function handleCriticalError(error: unknown): Promise { const [{ FatalError }, { AlreadyReportedError }] = await Promise.all([ - import('@qwen-code/qwen-code-core'), + import('./utils/deferred-core-runtime.js'), import('./utils/errors.js'), ]); diff --git a/packages/cli/src/serve/core-runtime.ts b/packages/cli/src/serve/core-runtime.ts new file mode 100644 index 00000000000..9ab9c48fb53 --- /dev/null +++ b/packages/cli/src/serve/core-runtime.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export { + DEFAULT_OTLP_ENDPOINT, + DEFAULT_TELEMETRY_TARGET, + FatalConfigError, + Storage, + applyProviderInstallPlan, + buildInstallPlan, + createDaemonBridgeTelemetry, + emitDaemonLog, + findProviderById, + forceFlushMetrics, + getDefaultModelIds, + hashDaemonWorkspace, + initializeDaemonMetrics, + initializeTelemetry, + recordDaemonCancel, + recordDaemonChannelLifecycle, + recordDaemonPipeMessage, + recordDaemonPromptDuration, + recordDaemonPromptQueueWait, + recordDaemonSessionLifecycle, + registerDaemonEventLoopLagGauge, + registerDaemonGaugeCallbacks, + resolveBaseUrl, + resolveTelemetrySettings, + shutdownTelemetry, + startEventLoopLagMonitor, +} from '@qwen-code/qwen-code-core'; diff --git a/packages/cli/src/serve/fs/workspace-file-system.ts b/packages/cli/src/serve/fs/workspace-file-system.ts index bfb0229b6de..2188e604a04 100644 --- a/packages/cli/src/serve/fs/workspace-file-system.ts +++ b/packages/cli/src/serve/fs/workspace-file-system.ts @@ -19,9 +19,9 @@ import { glob as globAsync } from 'glob'; import { StandardFileSystemService, - decodeBufferWithEncodingInfo, + decodeBufferWithEncodingInfoAsync, detectLineEnding, - encodeTextFileContent, + encodeTextFileContentAsync, loadIgnoreRules, isWithinRoot, type Ignore, @@ -1398,7 +1398,7 @@ async function readTextSnapshotFromResolvedFile( }); } - const decoded = decodeBufferWithEncodingInfo(raw); + const decoded = await decodeBufferWithEncodingInfoAsync(raw); const startLineIndex = opts.line !== undefined ? opts.line - 1 : 0; const sliced = sliceDecodedText( decoded.content, @@ -1645,7 +1645,7 @@ async function writeEncodedTextTemp(input: { meta: ReadMeta; handle: Awaited>; }): Promise { - const buf = encodeTextFileContent( + const buf = await encodeTextFileContentAsync( input.targetPath, input.content, buildWriteMeta(input.meta), diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index a2f49fcfa3f..ca0f2e5538a 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -709,7 +709,7 @@ export interface RunHandle { close(): Promise; } -type CoreRuntime = typeof import('@qwen-code/qwen-code-core'); +type CoreRuntime = typeof import('./core-runtime.js'); type ProviderConfig = NonNullable>; type SettingsRuntime = typeof import('../config/settings.js'); type EnvironmentRuntime = typeof import('../config/environment.js'); @@ -945,7 +945,7 @@ function shouldPreheatBridge(deps: RunQwenServeDeps): boolean { let coreRuntimePromise: Promise | undefined; function loadCoreRuntime(): Promise { - coreRuntimePromise ??= import('@qwen-code/qwen-code-core'); + coreRuntimePromise ??= import('./core-runtime.js'); return coreRuntimePromise; } diff --git a/packages/cli/src/serve/workspace-registration-store.ts b/packages/cli/src/serve/workspace-registration-store.ts index 15d8e6cd5ed..59135329081 100644 --- a/packages/cli/src/serve/workspace-registration-store.ts +++ b/packages/cli/src/serve/workspace-registration-store.ts @@ -518,7 +518,9 @@ export class WorkspaceRegistrationStore { private async update( mutate: (snapshot: WorkspaceRegistrationSnapshot) => boolean, ): Promise { - const { atomicWriteFile } = await import('@qwen-code/qwen-code-core'); + const { atomicWriteFile } = await import( + '../utils/deferred-core-runtime.js' + ); return withInProcessLock(this.filePath, async () => { const lock = await acquireFileLock(this.filePath); let committed = false; diff --git a/packages/cli/src/ui/utils/export/collect.ts b/packages/cli/src/ui/utils/export/collect.ts index cc046fb8f42..28b3982a729 100644 --- a/packages/cli/src/ui/utils/export/collect.ts +++ b/packages/cli/src/ui/utils/export/collect.ts @@ -359,7 +359,9 @@ async function extractMetadata( // Get git repository name let gitRepo: string | undefined; if (cwd) { - const { getGitRepoName } = await import('@qwen-code/qwen-code-core'); + const { getGitRepoName } = await import( + '../../../utils/deferred-core-runtime.js' + ); gitRepo = getGitRepoName(cwd); } diff --git a/packages/cli/src/utils/cleanup.ts b/packages/cli/src/utils/cleanup.ts index 2df01928a5d..8a0c28e68de 100644 --- a/packages/cli/src/utils/cleanup.ts +++ b/packages/cli/src/utils/cleanup.ts @@ -109,7 +109,7 @@ export function _resetCleanupFunctionsForTest(): void { } export async function cleanupCheckpoints() { - const { Storage } = await import('@qwen-code/qwen-code-core'); + const { Storage } = await import('./deferred-core-runtime.js'); const storage = new Storage(process.cwd()); const tempDir = storage.getProjectTempDir(); const checkpointsDir = join(tempDir, 'checkpoints'); diff --git a/packages/cli/src/utils/deferred-core-runtime.ts b/packages/cli/src/utils/deferred-core-runtime.ts new file mode 100644 index 00000000000..42f33b9f6fc --- /dev/null +++ b/packages/cli/src/utils/deferred-core-runtime.ts @@ -0,0 +1,12 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export { + FatalError, + Storage, + atomicWriteFile, + getGitRepoName, +} from '@qwen-code/qwen-code-core'; diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 53201c3bd14..1995ef98f86 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -53,7 +53,6 @@ import { StandardFileSystemService, type FileEncodingType, } from '../services/fileSystemService.js'; -import { GitWorktreeService } from '../services/gitWorktreeService.js'; import { cleanupStaleAgentWorktrees } from '../services/worktreeCleanup.js'; import { CronScheduler, @@ -66,6 +65,7 @@ import { validateMemoryPressureConfig, type MemoryPressureConfig, } from '../services/memoryPressureMonitor.js'; +import { findGitRoot } from '../utils/gitUtils.js'; // Tools — only lightweight imports; tool classes are lazy-loaded via dynamic import import { @@ -2803,8 +2803,7 @@ export class Config { // subdir, not the repo root) always early-returned and the // sweep was permanently a no-op. Fast-bail still happens, just // against the *correct* directory. - const probe = new GitWorktreeService(this.targetDir); - const root = (await probe.getRepoTopLevel()) ?? this.targetDir; + const root = findGitRoot(this.targetDir) ?? this.targetDir; const worktreesDir = path.join(root, '.qwen', 'worktrees'); try { await fsPromises.access(worktreesDir); diff --git a/packages/core/src/extension/github.ts b/packages/core/src/extension/github.ts index b38a67a36a3..d9f23be5f66 100644 --- a/packages/core/src/extension/github.ts +++ b/packages/core/src/extension/github.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { simpleGit, type SimpleGit } from 'simple-git'; +import type { SimpleGit } from 'simple-git'; import { getErrorMessage } from '../utils/errors.js'; import * as os from 'node:os'; import * as https from 'node:https'; @@ -29,6 +29,7 @@ import { import { assertTarArchiveHasNoLinks } from './archive-safety.js'; import { resolveNetworkTarget } from './network-policy.js'; import { extractZipArchive } from './zip-extraction.js'; +import { loadSimpleGit } from '../utils/load-simple-git.js'; const debugLogger = createDebugLogger('EXT_GITHUB'); const SUPPORTED_ARCHIVE_EXTENSIONS = ['.tar.gz', '.zip'] as const; @@ -109,6 +110,7 @@ function getGitHubToken(): string | undefined { } async function assertPinnedGitSupported(): Promise { + const { simpleGit } = await loadSimpleGit(); const version = await simpleGit().version(); if ( version.major < MINIMUM_PINNED_GIT_VERSION.major || @@ -166,6 +168,7 @@ export async function cloneFromGit( ): Promise { const redactedSource = redactUrlCredentials(installMetadata.source); try { + const { simpleGit } = await loadSimpleGit(); let networkConfig: string[] = []; if (installMetadata.networkPolicy === 'public') { if (!/^https:/i.test(installMetadata.source)) { @@ -422,6 +425,7 @@ export async function checkForExtensionUpdate( } try { if (installMetadata.type === 'git') { + const { simpleGit } = await loadSimpleGit(); if (installMetadata.networkPolicy === 'public') { await assertPinnedGitSupported(); } diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 46586fe677b..74dc70a5b30 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -259,7 +259,10 @@ export * from './services/fileDiscoveryService.js'; export * from './services/fileHistoryService.js'; export * from './services/fileReadCache.js'; export * from './services/fileSystemService.js'; -export { decodeBufferWithEncodingInfo } from './utils/fileUtils.js'; +export { + decodeBufferWithEncodingInfo, + encodeTextFileContent, +} from './utils/sync-file-encoding.js'; export * from './services/gitWorktreeService.js'; export { DEFAULT_MAX_TOOL_CALLS_PER_TURN } from './services/loopDetectionService.js'; export * from './services/visionBridge/vision-bridge-service.js'; diff --git a/packages/core/src/services/fileSystemService.test.ts b/packages/core/src/services/fileSystemService.test.ts index 495f6696628..b682825c7ae 100644 --- a/packages/core/src/services/fileSystemService.test.ts +++ b/packages/core/src/services/fileSystemService.test.ts @@ -8,12 +8,12 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; import fs from 'node:fs/promises'; import { StandardFileSystemService, - encodeTextFileContent, needsUtf8Bom, resetUtf8BomCache, detectLineEnding, ensureCrlfLineEndings, } from './fileSystemService.js'; +import { encodeTextFileContent } from '../utils/sync-file-encoding.js'; const mockPlatform = vi.hoisted(() => vi.fn().mockReturnValue('linux')); const mockGetSystemEncoding = vi.hoisted(() => diff --git a/packages/core/src/services/fileSystemService.ts b/packages/core/src/services/fileSystemService.ts index 62189c7d36b..ab5860cd941 100644 --- a/packages/core/src/services/fileSystemService.ts +++ b/packages/core/src/services/fileSystemService.ts @@ -10,11 +10,8 @@ import * as path from 'node:path'; import { globSync } from 'glob'; import { atomicWriteFile } from '../utils/atomicFileWrite.js'; import { readFileWithLineAndLimit } from '../utils/fileUtils.js'; -import { - iconvEncode, - iconvEncodingExists, - isUtf8CompatibleEncoding, -} from '../utils/iconvHelper.js'; +import { isUtf8CompatibleEncoding } from '../utils/encoding.js'; +import { loadIconvLite, type IconvLite } from '../utils/load-iconv-lite.js'; import { getSystemEncoding } from '../utils/systemEncoding.js'; import type { ReadTextFileRequest, @@ -182,7 +179,7 @@ export function detectLineEnding(content: string): LineEnding { return content.includes('\r\n') ? 'crlf' : 'lf'; } -interface PreparedTextFileContent { +export interface PreparedTextFileContent { data: string | Buffer; encoding?: BufferEncoding; } @@ -212,11 +209,12 @@ function getBOMBytesForEncoding(encoding: string): Buffer | null { } } -function prepareTextFileContent( +export function prepareTextFileContent( filePath: string, content: string, meta?: ReadTextFileResponse['_meta'] | null, -): PreparedTextFileContent { + iconvLite?: IconvLite, +): PreparedTextFileContent | undefined { const lineEnding = meta?.['lineEnding'] as string | undefined; const shouldUseCrlf = needsCrlfLineEndings(filePath) || lineEnding === 'crlf'; const normalizedContent = shouldUseCrlf @@ -226,17 +224,20 @@ function prepareTextFileContent( const encoding = meta?.['encoding'] as string | undefined; // Check if a non-UTF-8 encoding is specified and supported by iconv-lite - const isNonUtf8Encoding = + if (encoding && !isUtf8CompatibleEncoding(encoding) && !iconvLite) { + return undefined; + } + + if ( encoding && !isUtf8CompatibleEncoding(encoding) && - iconvEncodingExists(encoding); - - if (isNonUtf8Encoding) { + iconvLite?.encodingExists(encoding) + ) { // Non-UTF-8 encoding (e.g. GBK, Big5, Shift_JIS, UTF-16LE, UTF-32BE…) // Use iconv-lite to encode the content. When the file originally had a BOM // (bom: true), prepend the correct BOM bytes for this encoding so the // byte-order mark is preserved on write-back. - const encoded = iconvEncode(normalizedContent, encoding); + const encoded = iconvLite.encode(normalizedContent, encoding); if (bom) { const bomBytes = getBOMBytesForEncoding(encoding); return { @@ -261,14 +262,35 @@ function prepareTextFileContent( return { data: normalizedContent, encoding: 'utf-8' }; } -export function encodeTextFileContent( +export async function prepareTextFileContentAsync( + filePath: string, + content: string, + meta?: ReadTextFileResponse['_meta'] | null, +): Promise { + let prepared = prepareTextFileContent(filePath, content, meta); + if (!prepared) { + prepared = prepareTextFileContent( + filePath, + content, + meta, + await loadIconvLite(), + ); + } + if (!prepared) { + throw new Error('iconv-lite did not prepare non-UTF-8 text content'); + } + return prepared; +} + +export async function encodeTextFileContentAsync( filePath: string, content: string, meta?: ReadTextFileResponse['_meta'] | null, -): Buffer { - const prepared = prepareTextFileContent(filePath, content, meta); - if (Buffer.isBuffer(prepared.data)) return prepared.data; - return Buffer.from(prepared.data, prepared.encoding ?? 'utf-8'); +): Promise { + const prepared = await prepareTextFileContentAsync(filePath, content, meta); + return Buffer.isBuffer(prepared.data) + ? prepared.data + : Buffer.from(prepared.data, prepared.encoding ?? 'utf-8'); } /** @@ -308,7 +330,11 @@ export class StandardFileSystemService implements FileSystemService { params: Omit, ): Promise { const { path: filePath, _meta } = params; - const prepared = prepareTextFileContent(filePath, params.content, _meta); + const prepared = await prepareTextFileContentAsync( + filePath, + params.content, + _meta, + ); if (Buffer.isBuffer(prepared.data)) { await atomicWriteFile(filePath, prepared.data); } else { diff --git a/packages/core/src/services/gitWorktreeService.ts b/packages/core/src/services/gitWorktreeService.ts index 53a1b2d8e7c..dc8c17b0a4c 100644 --- a/packages/core/src/services/gitWorktreeService.ts +++ b/packages/core/src/services/gitWorktreeService.ts @@ -11,13 +11,13 @@ import { execFile, execSync } from 'node:child_process'; import { promisify } from 'node:util'; const execFileAsync = promisify(execFile); -import { simpleGit, CheckRepoActions } from 'simple-git'; import type { SimpleGit } from 'simple-git'; import { Storage } from '../config/storage.js'; import { isCommandAvailable } from '../utils/shell-utils.js'; import { isNodeError } from '../utils/errors.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { fileExists, isWithinRoot } from '../utils/fileUtils.js'; +import { loadSimpleGit } from '../utils/load-simple-git.js'; import { initRepositoryWithMainBranch } from './gitInit.js'; const debugLogger = createDebugLogger('GIT_WORKTREE_SERVICE'); @@ -57,6 +57,7 @@ export async function writeWorktreeSessionMarker( // `/.git/worktrees//`, so resolve `--git-dir` instead // of joining naively. try { + const { simpleGit } = await loadSimpleGit(); const wtGit = simpleGit(worktreePath); const gitDir = (await wtGit.revparse(['--git-dir'])).trim(); const excludePath = path.isAbsolute(gitDir) @@ -220,15 +221,21 @@ interface SessionConfigFile { */ export class GitWorktreeService { private sourceRepoPath: string; - private git: SimpleGit; + private gitPromise: Promise | undefined; private readonly customBaseDir?: string; constructor(sourceRepoPath: string, customBaseDir?: string) { this.sourceRepoPath = path.resolve(sourceRepoPath); - this.git = simpleGit(this.sourceRepoPath); this.customBaseDir = customBaseDir; } + private getGit(): Promise { + this.gitPromise ??= loadSimpleGit().then(({ simpleGit }) => + simpleGit(this.sourceRepoPath), + ); + return this.gitPromise; + } + /** * Gets the directory where worktrees are stored. * @param customDir - Optional custom base directory override @@ -293,7 +300,7 @@ export class GitWorktreeService { */ async getRepoTopLevel(): Promise { try { - const out = await this.git.revparse(['--show-toplevel']); + const out = await (await this.getGit()).revparse(['--show-toplevel']); const top = out.trim(); return top.length > 0 ? top : null; } catch (error) { @@ -313,16 +320,24 @@ export class GitWorktreeService { */ async isGitRepository(): Promise { try { - const isRoot = await this.git.checkIsRepo(CheckRepoActions.IS_REPO_ROOT); - if (isRoot) { - return true; + const [git, { CheckRepoActions }] = await Promise.all([ + this.getGit(), + loadSimpleGit(), + ]); + try { + const isRoot = await git.checkIsRepo(CheckRepoActions.IS_REPO_ROOT); + if (isRoot) { + return true; + } + } catch { + // IS_REPO_ROOT check failed — fall through to the general check + } + // Not the root (or root check threw) — check if we're inside a git repo + try { + return await git.checkIsRepo(); + } catch { + return false; } - } catch { - // IS_REPO_ROOT check failed — fall through to the general check - } - // Not the root (or root check threw) — check if we're inside a git repo - try { - return await this.git.checkIsRepo(); } catch { return false; } @@ -342,11 +357,12 @@ export class GitWorktreeService { } try { - await initRepositoryWithMainBranch(this.git); + const git = await this.getGit(); + await initRepositoryWithMainBranch(git); // Create initial commit so we can create worktrees - await this.git.add('.'); - await this.git.commit('Initial commit', { + await git.add('.'); + await git.commit('Initial commit', { '--allow-empty': null, }); @@ -363,7 +379,9 @@ export class GitWorktreeService { * Gets the current branch name. */ async getCurrentBranch(): Promise { - const branch = await this.git.revparse(['--abbrev-ref', 'HEAD']); + const branch = await ( + await this.getGit() + ).revparse(['--abbrev-ref', 'HEAD']); return branch.trim(); } @@ -371,7 +389,7 @@ export class GitWorktreeService { * Gets the current commit hash. */ async getCurrentCommitHash(): Promise { - const hash = await this.git.revparse(['HEAD']); + const hash = await (await this.getGit()).revparse(['HEAD']); return hash.trim(); } @@ -388,7 +406,9 @@ export class GitWorktreeService { */ async resolveRef(ref: string): Promise { try { - const out = (await this.git.raw(['rev-parse', '--verify', ref])).trim(); + const out = ( + await (await this.getGit()).raw(['rev-parse', '--verify', ref]) + ).trim(); return /^[0-9a-f]{40}$/.test(out) ? out : null; } catch { return null; @@ -429,14 +449,9 @@ export class GitWorktreeService { const branchName = `${base}-${shortSession}-${sanitizedName}`; // Create the worktree with a new branch - await this.git.raw([ - 'worktree', - 'add', - '-b', - branchName, - worktreePath, - base, - ]); + await ( + await this.getGit() + ).raw(['worktree', 'add', '-b', branchName, worktreePath, base]); const worktree: WorktreeInfo = { id: `${sessionId}/${sanitizedName}`, @@ -538,7 +553,9 @@ export class GitWorktreeService { // untracked files are handled separately below via file copy. let dirtyStateSnapshot = ''; try { - dirtyStateSnapshot = (await this.git.stash(['create'])).trim(); + dirtyStateSnapshot = ( + await (await this.getGit()).stash(['create']) + ).trim(); } catch { // Ignore — proceed without dirty state if stash create fails } @@ -547,11 +564,9 @@ export class GitWorktreeService { // `git ls-files --others --exclude-standard` is read-only and safe. let untrackedFiles: string[] = []; try { - const raw = await this.git.raw([ - 'ls-files', - '--others', - '--exclude-standard', - ]); + const raw = await ( + await this.getGit() + ).raw(['ls-files', '--others', '--exclude-standard']); untrackedFiles = raw.trim().split('\n').filter(Boolean); } catch { // Non-fatal: proceed without untracked files @@ -597,6 +612,7 @@ export class GitWorktreeService { // see the same files the user currently has on disk. if (result.success) { for (const worktree of result.worktrees) { + const { simpleGit } = await loadSimpleGit(); const wtGit = simpleGit(worktree.path); // 1. Apply tracked dirty changes (staged + unstaged) @@ -702,16 +718,26 @@ export class GitWorktreeService { async removeWorktree( worktreePath: string, ): Promise<{ success: boolean; error?: string }> { + let git: SimpleGit; + try { + git = await this.getGit(); + } catch (error) { + return { + success: false, + error: `Failed to remove worktree: ${error instanceof Error ? error.message : 'Unknown error'}`, + }; + } + try { // Remove the worktree from git - await this.git.raw(['worktree', 'remove', worktreePath, '--force']); + await git.raw(['worktree', 'remove', worktreePath, '--force']); return { success: true }; } catch (error) { // Try to remove the directory manually if git worktree remove fails try { await fs.rm(worktreePath, { recursive: true, force: true }); // Prune worktree references - await this.git.raw(['worktree', 'prune']); + await git.raw(['worktree', 'prune']); return { success: true }; } catch (_rmError) { return { @@ -772,23 +798,18 @@ export class GitWorktreeService { // Clean up branches that belonged to the worktrees try { + const git = await this.getGit(); for (const branchName of worktreeBranches) { try { - await this.git.branch(['-D', branchName]); + await git.branch(['-D', branchName]); result.removedBranches.push(branchName); } catch { // Branch might already be deleted, ignore } } + await git.raw(['worktree', 'prune']); } catch { - // Ignore branch listing/deletion errors - } - - // Prune worktree references - try { - await this.git.raw(['worktree', 'prune']); - } catch { - // Ignore prune errors + // Ignore branch deletion, loader, and prune errors } return result; @@ -804,14 +825,13 @@ export class GitWorktreeService { worktreePath: string, baseBranch?: string, ): Promise { - const worktreeGit = simpleGit(worktreePath); - - const base = - (await this.resolveBaseline(worktreeGit)) ?? - baseBranch ?? - (await this.getCurrentBranch()); - try { + const { simpleGit } = await loadSimpleGit(); + const worktreeGit = simpleGit(worktreePath); + const base = + (await this.resolveBaseline(worktreeGit)) ?? + baseBranch ?? + (await this.getCurrentBranch()); return await this.withStagedChanges(worktreeGit, () => worktreeGit.diff(['--binary', '--cached', base]), ); @@ -832,10 +852,11 @@ export class GitWorktreeService { targetPath?: string, ): Promise<{ success: boolean; error?: string }> { const target = targetPath || this.sourceRepoPath; - const worktreeGit = simpleGit(worktreePath); - const targetGit = simpleGit(target); try { + const { simpleGit } = await loadSimpleGit(); + const worktreeGit = simpleGit(worktreePath); + const targetGit = simpleGit(target); // Prefer the baseline commit (created during worktree setup after // overlaying dirty state) so the patch excludes pre-existing edits. let base = await this.resolveBaseline(worktreeGit); @@ -1145,18 +1166,19 @@ export class GitWorktreeService { } // Run the two probes in parallel: this repo's common-dir comes from - // `this.git`, the candidate's HEAD-SHA + branch + common-dir + + // the source-repo client, the candidate's HEAD-SHA + branch + common-dir + // toplevel come from a fresh simple-git rooted at `worktreePath` // via a single combined rev-parse. - const probeGit = simpleGit(worktreePath); let ourCommonDir: string; let headCommit: string; let branch: string; let probeCommonDir: string; let probeToplevel: string; try { + const { simpleGit } = await loadSimpleGit(); + const probeGit = simpleGit(worktreePath); const [ourRaw, probeRaw] = await Promise.all([ - this.git.raw(['rev-parse', '--git-common-dir']), + (await this.getGit()).raw(['rev-parse', '--git-common-dir']), probeGit.raw([ 'rev-parse', 'HEAD', @@ -1255,7 +1277,9 @@ export class GitWorktreeService { // controls candidate-side files, which are never consulted. const ourCommonDir = path.resolve( this.sourceRepoPath, - (await this.git.raw(['rev-parse', '--git-common-dir'])).trim(), + ( + await (await this.getGit()).raw(['rev-parse', '--git-common-dir']) + ).trim(), ); const worktreesDir = path.join(ourCommonDir, 'worktrees'); let entryNames: string[]; @@ -1293,6 +1317,7 @@ export class GitWorktreeService { // resolves it into the MAIN checkout, so its `--git-dir` will not be this // entry. The registry answers "is this path registered?"; only a probe // inside the path answers "is it a worktree right now?". + const { simpleGit } = await loadSimpleGit(); const rawGitDir = ( await simpleGit(target).raw(['rev-parse', '--git-dir']) ).trim(); @@ -1553,14 +1578,9 @@ export class GitWorktreeService { return { success: false, error }; } - await this.git.raw([ - 'worktree', - 'add', - '-b', - branchName, - worktreePath, - base, - ]); + await ( + await this.getGit() + ).raw(['worktree', 'add', '-b', branchName, worktreePath, base]); // Configure core.hooksPath so commits inside the worktree run the // main repo's hooks (the new worktree's .git directory has no hooks @@ -1643,7 +1663,7 @@ export class GitWorktreeService { if (!hooksPath) { try { const commonDir = ( - await this.git.raw(['rev-parse', '--git-common-dir']) + await (await this.getGit()).raw(['rev-parse', '--git-common-dir']) ).trim(); const resolvedCommonDir = path.isAbsolute(commonDir) ? commonDir @@ -1661,6 +1681,7 @@ export class GitWorktreeService { } if (!hooksPath) return; + const { simpleGit } = await loadSimpleGit(); const worktreeGit = simpleGit(worktreePath, { unsafe: { allowUnsafeHooksPath: true }, }); @@ -1978,7 +1999,9 @@ export class GitWorktreeService { */ private async localBranchExists(branchName: string): Promise { try { - const out = await this.git.raw([ + const out = await ( + await this.getGit() + ).raw([ 'for-each-ref', '--count=1', '--format=%(refname)', @@ -2066,8 +2089,9 @@ export class GitWorktreeService { // remove branches whose tip is not reachable from HEAD or any // upstream — preserving any commits the subagent made before // ending with a clean working tree. + const git = await this.getGit(); try { - await this.git.branch(['-d', branchName]); + await git.branch(['-d', branchName]); return { success: true }; } catch (error) { // Refused either because the branch carries unmerged commits @@ -2082,7 +2106,7 @@ export class GitWorktreeService { if (options.forceDeleteBranch) { try { - await this.git.branch(['-D', branchName]); + await git.branch(['-D', branchName]); return { success: true }; } catch (error) { // Best-effort: branch may have been deleted already, or may not @@ -2114,13 +2138,14 @@ export class GitWorktreeService { async hasUnmergedWorktreeCommits(slug: string): Promise { const branchName = worktreeBranchForSlug(slug); try { - const tipSha = (await this.git.revparse([branchName])).trim(); + const git = await this.getGit(); + const tipSha = (await git.revparse([branchName])).trim(); if (!tipSha) return true; // List every local branch and remote-tracking ref whose tip is at // or above the worktree branch's tip. If anything other than the // worktree branch itself appears, the commits are covered. const refs = ( - await this.git.raw([ + await git.raw([ 'for-each-ref', '--contains', tipSha, @@ -2155,6 +2180,7 @@ export class GitWorktreeService { */ async hasWorktreeChanges(worktreePath: string): Promise { try { + const { simpleGit } = await loadSimpleGit(); const wtGit = simpleGit(worktreePath); const status = await wtGit.status(); // Defensive: `status.isClean()` reads several status arrays, but @@ -2178,6 +2204,7 @@ export class GitWorktreeService { worktreePath: string, ): Promise<{ tracked: number; untracked: number } | null> { try { + const { simpleGit } = await loadSimpleGit(); const wtGit = simpleGit(worktreePath); const status = await wtGit.status(); // `conflicted` is mutually exclusive with the other arrays in diff --git a/packages/core/src/services/shellExecutionService.ts b/packages/core/src/services/shellExecutionService.ts index 9a573c6b474..56989784a77 100644 --- a/packages/core/src/services/shellExecutionService.ts +++ b/packages/core/src/services/shellExecutionService.ts @@ -11,10 +11,14 @@ import { spawn as cpSpawn, spawnSync } from 'node:child_process'; import { TextDecoder } from 'node:util'; import os from 'node:os'; import type { IPty } from '@lydell/node-pty'; +import type { Terminal } from '@xterm/headless'; import { getCachedEncodingForBuffer } from '../utils/systemEncoding.js'; import { isBinary } from '../utils/textUtils.js'; import { getShellConfiguration, type ShellType } from '../utils/shell-utils.js'; -import pkg from '@xterm/headless'; +import { + loadXtermHeadless, + type XtermHeadlessModule, +} from '../utils/load-xterm-headless.js'; import { serializeTerminalToObject, serializeTerminalToText, @@ -26,7 +30,6 @@ import { formatMemoryUsage } from '../utils/formatters.js'; import { getShellContextEnvVars } from '../utils/shellContextEnv.js'; import { createDebugLogger } from '../utils/debugLogger.js'; import { getShellPagerEnv } from '../utils/shell-pager-env.js'; -const { Terminal } = pkg; const debugLogger = createDebugLogger('SHELL_EXECUTION'); @@ -347,7 +350,7 @@ export type ShellOutputEvent = interface ActivePty { ptyProcess: IPty; - headlessTerminal: pkg.Terminal; + headlessTerminal: Terminal; } const getErrnoCode = (error: unknown): string | undefined => { @@ -388,6 +391,7 @@ const replayTerminalOutput = async ( output: string, cols: number, rows: number, + Terminal: XtermHeadlessModule['Terminal'], ): Promise => { const replayTerminal = new Terminal({ allowProposedApi: true, @@ -470,7 +474,7 @@ const createPlainAnsiLine = (text: string) => [ ]; const serializePlainViewportToAnsiOutput = ( - terminal: pkg.Terminal, + terminal: Terminal, unwrapWrappedLines = false, ): AnsiOutput => { const buffer = terminal.buffer.active; @@ -708,6 +712,10 @@ export class ShellExecutionService { const ptyInfo = ptyOutcome.value; if (ptyInfo) { try { + const { Terminal } = await loadXtermHeadless(); + if (abortSignal.aborted) { + return createPreSpawnAbortedHandle(); + } return this.executeWithPty( commandToExecute, cwd, @@ -715,6 +723,7 @@ export class ShellExecutionService { abortSignal, shellExecutionConfig, ptyInfo, + Terminal, options.postPromote, ); } catch (_e) { @@ -1435,6 +1444,7 @@ export class ShellExecutionService { abortSignal: AbortSignal, shellExecutionConfig: ShellExecutionConfig, ptyInfo: PtyImplementation, + Terminal: XtermHeadlessModule['Terminal'], postPromote?: ShellPostPromoteHandlers, ): ShellExecutionHandle { if (!ptyInfo) { @@ -1851,6 +1861,7 @@ export class ShellExecutionService { decodedOutput, cols, rows, + Terminal, ); } else { fullOutput = serializeTerminalToText(headlessTerminal); @@ -2201,7 +2212,12 @@ export class ShellExecutionService { const decodedOutput = new TextDecoder(finalEncoding).decode( finalBuffer, ); - snapshot = await replayTerminalOutput(decodedOutput, cols, rows); + snapshot = await replayTerminalOutput( + decodedOutput, + cols, + rows, + Terminal, + ); } else { snapshot = serializeTerminalToText(headlessTerminal) ?? ''; } diff --git a/packages/core/src/services/worktreeCleanup.ts b/packages/core/src/services/worktreeCleanup.ts index 3c0fe335c64..63f0df98483 100644 --- a/packages/core/src/services/worktreeCleanup.ts +++ b/packages/core/src/services/worktreeCleanup.ts @@ -6,13 +6,13 @@ import * as fs from 'node:fs/promises'; import * as path from 'node:path'; -import { simpleGit } from 'simple-git'; import { AGENT_WORKTREE_SLUG_PATTERN, GitWorktreeService, worktreeBranchForSlug, } from './gitWorktreeService.js'; import { createDebugLogger } from '../utils/debugLogger.js'; +import { loadSimpleGit } from '../utils/load-simple-git.js'; const debugLogger = createDebugLogger('WORKTREE_CLEANUP'); @@ -155,6 +155,7 @@ export async function cleanupStaleAgentWorktrees( async function hasTrackedChanges(worktreePath: string): Promise { try { + const { simpleGit } = await loadSimpleGit(); const wtGit = simpleGit(worktreePath); // `git status --porcelain --untracked-files=no` lists every tracked // change (staged, unstaged, conflicted — `UU` lines) and skips the diff --git a/packages/core/src/utils/encoding.ts b/packages/core/src/utils/encoding.ts new file mode 100644 index 00000000000..e4ee3ea347a --- /dev/null +++ b/packages/core/src/utils/encoding.ts @@ -0,0 +1,10 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export function isUtf8CompatibleEncoding(encoding: string): boolean { + const lower = encoding.toLowerCase().replace(/[^a-z0-9]/g, ''); + return lower === 'utf8' || lower === 'ascii' || lower === 'usascii'; +} diff --git a/packages/core/src/utils/fileUtils.test.ts b/packages/core/src/utils/fileUtils.test.ts index 274d11bfac1..865bc46340e 100644 --- a/packages/core/src/utils/fileUtils.test.ts +++ b/packages/core/src/utils/fileUtils.test.ts @@ -29,13 +29,13 @@ import { detectFileType, processSingleFileContent, detectBOM, - decodeBufferWithEncodingInfo, readFileWithLineAndLimit, readFileWithEncoding, readFileWithEncodingInfo, detectFileEncoding, fileExists, } from './fileUtils.js'; +import { decodeBufferWithEncodingInfo } from './sync-file-encoding.js'; import { iconvEncode } from './iconvHelper.js'; import { LargeNonUtf8TextError } from './read-text-range.js'; import type { Config } from '../config/config.js'; diff --git a/packages/core/src/utils/fileUtils.ts b/packages/core/src/utils/fileUtils.ts index 327801d76da..67bd88b0e8d 100644 --- a/packages/core/src/utils/fileUtils.ts +++ b/packages/core/src/utils/fileUtils.ts @@ -9,11 +9,8 @@ import fsPromises from 'node:fs/promises'; import path from 'node:path'; import type { Part, PartListUnion } from '@google/genai'; import mime from 'mime/lite'; -import { - iconvDecode, - iconvEncodingExists, - isUtf8CompatibleEncoding, -} from './iconvHelper.js'; +import { isUtf8CompatibleEncoding } from './encoding.js'; +import { loadIconvLite } from './load-iconv-lite.js'; import { ToolErrorType } from '../tools/tool-error.js'; import { BINARY_EXTENSIONS } from './ignorePatterns.js'; import type { Config } from '../config/config.js'; @@ -60,9 +57,14 @@ const PDF_PAGED_TEXT_EXTRACTION_MAX_MB = 512; // --- Unicode BOM detection & decoding helpers -------------------------------- -type UnicodeEncoding = 'utf8' | 'utf16le' | 'utf16be' | 'utf32le' | 'utf32be'; +export type UnicodeEncoding = + | 'utf8' + | 'utf16le' + | 'utf16be' + | 'utf32le' + | 'utf32be'; -interface BOMInfo { +export interface BOMInfo { encoding: UnicodeEncoding; bomLength: number; } @@ -160,7 +162,7 @@ function decodeUTF32(buf: Buffer, littleEndian: boolean): string { * Check whether a buffer is valid UTF-8 by attempting a strict decode. * If any invalid byte sequence is encountered, TextDecoder with `fatal: true` throws. */ -function isValidUtf8(buffer: Buffer): boolean { +export function isValidUtf8(buffer: Buffer): boolean { try { new TextDecoder('utf-8', { fatal: true }).decode(buffer); return true; @@ -185,7 +187,9 @@ export interface FileReadResult { bom: boolean; } -export function decodeBufferWithEncodingInfo(full: Buffer): FileReadResult { +export async function decodeBufferWithEncodingInfoAsync( + full: Buffer, +): Promise { if (full.length === 0) { return { content: '', encoding: 'utf-8', bom: false }; } @@ -210,9 +214,10 @@ export function decodeBufferWithEncodingInfo(full: Buffer): FileReadResult { const detected = detectEncodingFromBuffer(full); if (detected && !isUtf8CompatibleEncoding(detected)) { try { - if (iconvEncodingExists(detected)) { + const iconvLite = await loadIconvLite(); + if (iconvLite.encodingExists(detected)) { return { - content: iconvDecode(full, detected), + content: iconvLite.decode(full, detected), encoding: detected, bom: false, }; @@ -232,7 +237,7 @@ export function decodeBufferWithEncodingInfo(full: Buffer): FileReadResult { * Internal helper: decode a buffer given a BOMInfo. * Returns the decoded string for each supported BOM encoding. */ -function decodeBOMBuffer(buf: Buffer, bomInfo: BOMInfo): string { +export function decodeBOMBuffer(buf: Buffer, bomInfo: BOMInfo): string { const content = buf.subarray(bomInfo.bomLength); switch (bomInfo.encoding) { case 'utf8': @@ -254,7 +259,7 @@ function decodeBOMBuffer(buf: Buffer, bomInfo: BOMInfo): string { /** * Map a BOMInfo encoding to a canonical encoding name string. */ -function bomEncodingToName(bomEncoding: UnicodeEncoding): string { +export function bomEncodingToName(bomEncoding: UnicodeEncoding): string { switch (bomEncoding) { case 'utf8': return 'utf-8'; @@ -289,7 +294,7 @@ export async function readFileWithEncodingInfo( filePath, signal === undefined ? undefined : { signal }, ); - return decodeBufferWithEncodingInfo(full); + return await decodeBufferWithEncodingInfoAsync(full); } /** diff --git a/packages/core/src/utils/iconvHelper.ts b/packages/core/src/utils/iconvHelper.ts index 12c1a56c825..7cb90cb32dc 100644 --- a/packages/core/src/utils/iconvHelper.ts +++ b/packages/core/src/utils/iconvHelper.ts @@ -23,6 +23,8 @@ interface IconvLite { import iconvModule from 'iconv-lite'; const iconvLite: IconvLite = iconvModule as unknown as IconvLite; +export { isUtf8CompatibleEncoding } from './encoding.js'; + /** * Decode a buffer using the specified encoding. * @param buffer The buffer to decode @@ -59,7 +61,3 @@ export function iconvEncodingExists(encoding: string): boolean { * @param encoding The encoding name to check * @returns True if the encoding is UTF-8 or ASCII compatible */ -export function isUtf8CompatibleEncoding(encoding: string): boolean { - const lower = encoding.toLowerCase().replace(/[^a-z0-9]/g, ''); - return lower === 'utf8' || lower === 'ascii' || lower === 'usascii'; -} diff --git a/packages/core/src/utils/load-iconv-lite.test.ts b/packages/core/src/utils/load-iconv-lite.test.ts new file mode 100644 index 00000000000..706cf69bb11 --- /dev/null +++ b/packages/core/src/utils/load-iconv-lite.test.ts @@ -0,0 +1,30 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +describe('loadIconvLite', () => { + afterEach(() => { + vi.doUnmock('iconv-lite'); + vi.resetModules(); + }); + + it('unwraps the CommonJS default and single-flights concurrent loads', async () => { + const iconvLite = { + decode: vi.fn(), + encode: vi.fn(), + encodingExists: vi.fn(), + }; + vi.doMock('iconv-lite', () => ({ default: iconvLite })); + const { loadIconvLite } = await import('./load-iconv-lite.js'); + + const first = loadIconvLite(); + const second = loadIconvLite(); + + expect(second).toBe(first); + await expect(first).resolves.toBe(iconvLite); + }); +}); diff --git a/packages/core/src/utils/load-iconv-lite.ts b/packages/core/src/utils/load-iconv-lite.ts new file mode 100644 index 00000000000..69ba091bf77 --- /dev/null +++ b/packages/core/src/utils/load-iconv-lite.ts @@ -0,0 +1,20 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +export interface IconvLite { + decode(buffer: Buffer, encoding: string): string; + encode(content: string, encoding: string): Buffer; + encodingExists(encoding: string): boolean; +} + +let iconvLiteModulePromise: Promise | undefined; + +export function loadIconvLite(): Promise { + iconvLiteModulePromise ??= import('iconv-lite').then( + (module) => module.default as unknown as IconvLite, + ); + return iconvLiteModulePromise; +} diff --git a/packages/core/src/utils/load-simple-git.test.ts b/packages/core/src/utils/load-simple-git.test.ts new file mode 100644 index 00000000000..eba9a92f4b3 --- /dev/null +++ b/packages/core/src/utils/load-simple-git.test.ts @@ -0,0 +1,58 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +describe('loadSimpleGit', () => { + afterEach(() => { + vi.doUnmock('simple-git'); + vi.resetModules(); + }); + + it('uses named exports and single-flights concurrent loads', async () => { + const simpleGit = vi.fn(); + const CheckRepoActions = { IS_REPO_ROOT: 'root' }; + vi.doMock('simple-git', () => ({ CheckRepoActions, simpleGit })); + const { loadSimpleGit } = await import('./load-simple-git.js'); + + const first = loadSimpleGit(); + const second = loadSimpleGit(); + + expect(second).toBe(first); + await expect(first).resolves.toEqual({ CheckRepoActions, simpleGit }); + }); + + it('unwraps a default-only CommonJS chunk', async () => { + const simpleGit = Object.assign(vi.fn(), { + CheckRepoActions: { IS_REPO_ROOT: 'root' }, + }); + Object.assign(simpleGit, { simpleGit }); + vi.doMock('simple-git', () => ({ + CheckRepoActions: undefined, + simpleGit: undefined, + default: simpleGit, + })); + const { loadSimpleGit } = await import('./load-simple-git.js'); + + await expect(loadSimpleGit()).resolves.toEqual({ + CheckRepoActions: simpleGit.CheckRepoActions, + simpleGit, + }); + }); + + it('rejects an unexpected module shape', async () => { + vi.doMock('simple-git', () => ({ + CheckRepoActions: undefined, + simpleGit: undefined, + default: undefined, + })); + const { loadSimpleGit } = await import('./load-simple-git.js'); + + await expect(loadSimpleGit()).rejects.toThrow( + 'simple-git module does not match the expected API', + ); + }); +}); diff --git a/packages/core/src/utils/load-simple-git.ts b/packages/core/src/utils/load-simple-git.ts new file mode 100644 index 00000000000..40ac04e3263 --- /dev/null +++ b/packages/core/src/utils/load-simple-git.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { CheckRepoActions, SimpleGitFactory } from 'simple-git'; + +export type SimpleGitModule = { + CheckRepoActions: typeof CheckRepoActions; + simpleGit: SimpleGitFactory; +}; + +let simpleGitModulePromise: Promise | undefined; + +export function loadSimpleGit(): Promise { + simpleGitModulePromise ??= import('simple-git').then((module) => { + const imported = module as unknown as Partial & { + default?: SimpleGitModule; + }; + const candidate = + imported.simpleGit && imported.CheckRepoActions + ? (imported as SimpleGitModule) + : imported.default; + if (!candidate) { + throw new Error('simple-git module does not match the expected API'); + } + return { + CheckRepoActions: candidate.CheckRepoActions, + simpleGit: candidate.simpleGit, + }; + }); + return simpleGitModulePromise; +} diff --git a/packages/core/src/utils/load-xterm-headless.test.ts b/packages/core/src/utils/load-xterm-headless.test.ts new file mode 100644 index 00000000000..a9886ea2c63 --- /dev/null +++ b/packages/core/src/utils/load-xterm-headless.test.ts @@ -0,0 +1,34 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, describe, expect, it, vi } from 'vitest'; + +describe('loadXtermHeadless', () => { + afterEach(() => { + vi.doUnmock('@xterm/headless'); + vi.resetModules(); + }); + + it('uses named exports and single-flights concurrent loads', async () => { + class Terminal {} + vi.doMock('@xterm/headless', () => ({ Terminal })); + const { loadXtermHeadless } = await import('./load-xterm-headless.js'); + + const first = loadXtermHeadless(); + const second = loadXtermHeadless(); + + expect(second).toBe(first); + await expect(first).resolves.toEqual({ Terminal }); + }); + + it('unwraps a default-only CommonJS chunk', async () => { + class Terminal {} + vi.doMock('@xterm/headless', () => ({ default: { Terminal } })); + const { loadXtermHeadless } = await import('./load-xterm-headless.js'); + + await expect(loadXtermHeadless()).resolves.toEqual({ Terminal }); + }); +}); diff --git a/packages/core/src/utils/load-xterm-headless.ts b/packages/core/src/utils/load-xterm-headless.ts new file mode 100644 index 00000000000..eb4704b3920 --- /dev/null +++ b/packages/core/src/utils/load-xterm-headless.ts @@ -0,0 +1,24 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import type { Terminal } from '@xterm/headless'; + +export type XtermHeadlessModule = { + Terminal: typeof Terminal; +}; + +let xtermHeadlessModulePromise: Promise | undefined; + +export function loadXtermHeadless(): Promise { + xtermHeadlessModulePromise ??= import('@xterm/headless').then((module) => { + const candidate = + 'Terminal' in module + ? module + : (module as unknown as { default: XtermHeadlessModule }).default; + return candidate as XtermHeadlessModule; + }); + return xtermHeadlessModulePromise; +} diff --git a/packages/core/src/utils/read-text-range.ts b/packages/core/src/utils/read-text-range.ts index d6753728931..bb89a6c6f83 100644 --- a/packages/core/src/utils/read-text-range.ts +++ b/packages/core/src/utils/read-text-range.ts @@ -8,7 +8,7 @@ import { createReadStream, type Stats } from 'node:fs'; import { stat } from 'node:fs/promises'; import { TextDecoder } from 'node:util'; import { detectFileEncoding, readFileWithEncodingInfo } from './fileUtils.js'; -import { isUtf8CompatibleEncoding } from './iconvHelper.js'; +import { isUtf8CompatibleEncoding } from './encoding.js'; import { DEFAULT_RANGE_READ_BYTES, TEXT_RANGE_FAST_PATH_MAX_SIZE, diff --git a/packages/core/src/utils/sync-file-encoding.ts b/packages/core/src/utils/sync-file-encoding.ts new file mode 100644 index 00000000000..47a5cc09b65 --- /dev/null +++ b/packages/core/src/utils/sync-file-encoding.ts @@ -0,0 +1,82 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + iconvDecode, + iconvEncode, + iconvEncodingExists, +} from './iconvHelper.js'; +import { + bomEncodingToName, + decodeBOMBuffer, + detectBOM, + isValidUtf8, + type FileReadResult, +} from './fileUtils.js'; +import { + prepareTextFileContent, + type ReadTextFileResponse, +} from '../services/fileSystemService.js'; +import { detectEncodingFromBuffer } from './systemEncoding.js'; +import { isUtf8CompatibleEncoding } from './encoding.js'; +import { createDebugLogger } from './debugLogger.js'; + +const debugLogger = createDebugLogger('SYNC_FILE_ENCODING'); + +export function decodeBufferWithEncodingInfo(full: Buffer): FileReadResult { + if (full.length === 0) { + return { content: '', encoding: 'utf-8', bom: false }; + } + + const bomInfo = detectBOM(full); + if (bomInfo) { + return { + content: decodeBOMBuffer(full, bomInfo), + encoding: bomEncodingToName(bomInfo.encoding), + bom: true, + }; + } + + if (isValidUtf8(full)) { + return { content: full.toString('utf8'), encoding: 'utf-8', bom: false }; + } + + const detected = detectEncodingFromBuffer(full); + if (detected && !isUtf8CompatibleEncoding(detected)) { + try { + if (iconvEncodingExists(detected)) { + return { + content: iconvDecode(full, detected), + encoding: detected, + bom: false, + }; + } + } catch (error) { + debugLogger.warn( + `Failed to decode buffer as ${detected}: ${error instanceof Error ? error.message : String(error)}`, + ); + } + } + + return { content: full.toString('utf8'), encoding: 'utf-8', bom: false }; +} + +export function encodeTextFileContent( + filePath: string, + content: string, + meta?: ReadTextFileResponse['_meta'] | null, +): Buffer { + const prepared = prepareTextFileContent(filePath, content, meta, { + decode: iconvDecode, + encode: iconvEncode, + encodingExists: iconvEncodingExists, + }); + if (!prepared) { + throw new Error('iconv-lite did not prepare non-UTF-8 text content'); + } + if (Buffer.isBuffer(prepared.data)) return prepared.data; + return Buffer.from(prepared.data, prepared.encoding ?? 'utf-8'); +} diff --git a/scripts/check-serve-fast-path-bundle.js b/scripts/check-serve-fast-path-bundle.js index 79821ef2b73..897c5e15a42 100644 --- a/scripts/check-serve-fast-path-bundle.js +++ b/scripts/check-serve-fast-path-bundle.js @@ -217,6 +217,11 @@ const FORBIDDEN_ACP_PACKAGES = [ // first use (issue #7264 candidate 3). Keep the SDK out of the ACP bootstrap // closure. { label: 'Google GenAI SDK', packageName: '@google/genai' }, + // Encoding tables, terminal emulation, and git orchestration load at their + // first real use (issue #7264 candidate 5). + { label: 'iconv-lite encoding tables', packageName: 'iconv-lite' }, + { label: 'xterm headless runtime', packageName: '@xterm/headless' }, + { label: 'simple-git runtime', packageName: 'simple-git' }, ]; export function normalizeMetafilePath(filePath) { diff --git a/scripts/tests/serve-fast-path-bundle-check.test.js b/scripts/tests/serve-fast-path-bundle-check.test.js index 782950a3981..738f4987b13 100644 --- a/scripts/tests/serve-fast-path-bundle-check.test.js +++ b/scripts/tests/serve-fast-path-bundle-check.test.js @@ -503,6 +503,48 @@ describe('ACP import boundary check', () => { expect(findAcpImportBoundaryOffenders(metafile)).toEqual([]); }); + it('reports first-use dependency packages reached through static imports', () => { + const metafile = makeMetafile({ + 'dist/chunks/acp-agent.js': output({ + inputs: ['packages/cli/src/acp-integration/acpAgent.ts'], + imports: [staticImport('dist/chunks/first-use-dependencies.js')], + }), + 'dist/chunks/first-use-dependencies.js': output({ + inputs: [ + 'node_modules/iconv-lite/lib/index.js', + 'node_modules/@xterm/headless/lib-headless/xterm-headless.js', + 'node_modules/simple-git/dist/esm/index.js', + ], + }), + }); + + expect(findAcpImportBoundaryOffenders(metafile)).toEqual( + expect.arrayContaining([ + expect.objectContaining({ label: 'iconv-lite encoding tables' }), + expect.objectContaining({ label: 'xterm headless runtime' }), + expect.objectContaining({ label: 'simple-git runtime' }), + ]), + ); + }); + + it('allows first-use dependency packages behind dynamic imports', () => { + const metafile = makeMetafile({ + 'dist/chunks/acp-agent.js': output({ + inputs: ['packages/cli/src/acp-integration/acpAgent.ts'], + imports: [dynamicImport('dist/chunks/first-use-dependencies.js')], + }), + 'dist/chunks/first-use-dependencies.js': output({ + inputs: [ + 'node_modules/iconv-lite/lib/index.js', + 'node_modules/@xterm/headless/lib-headless/xterm-headless.js', + 'node_modules/simple-git/dist/esm/index.js', + ], + }), + }); + + expect(findAcpImportBoundaryOffenders(metafile)).toEqual([]); + }); + it('reads a metafile path and returns ACP boundary offenders', () => { const tempDir = mkdtempSync(join(tmpdir(), 'acp-import-boundary-')); try { From ac96c57b18ebf246995acb9eb818a50ed179b5b8 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 25 Jul 2026 00:50:24 +0800 Subject: [PATCH 2/5] test(core): Fix simple-git loader mock Co-authored-by: Qwen-Coder --- packages/core/src/extension/extensionManager.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/src/extension/extensionManager.test.ts b/packages/core/src/extension/extensionManager.test.ts index 7f9b801008b..92993a69190 100644 --- a/packages/core/src/extension/extensionManager.test.ts +++ b/packages/core/src/extension/extensionManager.test.ts @@ -46,6 +46,7 @@ const mockDownloadFromArchiveUrl = vi.hoisted(() => vi.fn()); const mockExtractArchiveFile = vi.hoisted(() => vi.fn()); vi.mock('simple-git', () => ({ + CheckRepoActions: { IS_REPO_ROOT: 'is-repo-root' }, simpleGit: vi.fn((path: string) => { mockGit.path.mockReturnValue(path); return mockGit; From 2208817aca16c677013adab7ce9067f1db7b8218 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 25 Jul 2026 08:58:32 +0800 Subject: [PATCH 3/5] test(core): Cover abort during xterm load Co-authored-by: Qwen-Coder --- .../services/shellExecutionService.test.ts | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/packages/core/src/services/shellExecutionService.test.ts b/packages/core/src/services/shellExecutionService.test.ts index 37ece77da29..d4d2cd1ce8f 100644 --- a/packages/core/src/services/shellExecutionService.test.ts +++ b/packages/core/src/services/shellExecutionService.test.ts @@ -42,6 +42,7 @@ const mockSpawnSync = vi.hoisted(() => vi.fn()); const mockIsBinary = vi.hoisted(() => vi.fn()); const mockPlatform = vi.hoisted(() => vi.fn()); const mockGetPty = vi.hoisted(() => vi.fn()); +const mockLoadXtermHeadless = vi.hoisted(() => vi.fn()); const mockSerializeTerminalToObject = vi.hoisted(() => vi.fn()); const mockSerializeTerminalToText = vi.hoisted(() => vi.fn((terminal: pkg.Terminal): string => { @@ -103,6 +104,9 @@ vi.mock('os', () => ({ vi.mock('../utils/getPty.js', () => ({ getPty: mockGetPty, })); +vi.mock('../utils/load-xterm-headless.js', () => ({ + loadXtermHeadless: mockLoadXtermHeadless, +})); vi.mock('../utils/terminalSerializer.js', () => ({ serializeTerminalToObject: mockSerializeTerminalToObject, serializeTerminalToText: mockSerializeTerminalToText, @@ -250,6 +254,7 @@ describe('ShellExecutionService', () => { module: { spawn: mockPtySpawn }, name: 'mock-pty', }); + mockLoadXtermHeadless.mockResolvedValue({ Terminal }); onOutputEventMock = vi.fn(); @@ -3504,6 +3509,42 @@ describe('ShellExecutionService execution method selection', () => { }, ); + it('does not spawn when aborted while xterm is loading', async () => { + let resolveXterm: + | ((value: { Terminal: typeof Terminal }) => void) + | undefined; + mockLoadXtermHeadless.mockReturnValue( + new Promise((resolve) => { + resolveXterm = resolve; + }), + ); + const abortController = new AbortController(); + const handlePromise = ShellExecutionService.execute( + 'test command', + '/test/dir', + onOutputEventMock, + abortController.signal, + true, + shellExecutionConfig, + ); + + await vi.waitFor(() => { + expect(mockLoadXtermHeadless).toHaveBeenCalledOnce(); + }); + abortController.abort(); + resolveXterm?.({ Terminal }); + + const handle = await handlePromise; + expect(await handle.result).toMatchObject({ + aborted: true, + pid: undefined, + executionMethod: 'none', + output: '', + }); + expect(mockPtySpawn).not.toHaveBeenCalled(); + expect(mockCpSpawn).not.toHaveBeenCalled(); + }); + it('should use node-pty when shouldUseNodePty is true and pty is available', async () => { const abortController = new AbortController(); const handle = await ShellExecutionService.execute( From 769ceac29733229b5fe12ca8973eb148fedf6408 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 25 Jul 2026 10:07:00 +0800 Subject: [PATCH 4/5] fix(core): Address lazy-loader review feedback Co-authored-by: Qwen-Coder --- esbuild.config.js | 2 +- packages/core/src/utils/iconvHelper.ts | 10 ---- .../core/src/utils/load-iconv-lite.test.ts | 21 ++++++++ packages/core/src/utils/load-iconv-lite.ts | 25 ++++++++-- .../core/src/utils/sync-file-encoding.test.ts | 50 +++++++++++++++++++ 5 files changed, 94 insertions(+), 14 deletions(-) create mode 100644 packages/core/src/utils/sync-file-encoding.test.ts diff --git a/esbuild.config.js b/esbuild.config.js index 613b8d462af..d1fe6b19534 100644 --- a/esbuild.config.js +++ b/esbuild.config.js @@ -123,7 +123,7 @@ const syncFileEncodingTreeShakePlugin = { { filter: /^\.\/utils\/sync-file-encoding\.js$/ }, (args) => { if ( - !/[\\/]packages[\\/]core[\\/](?:src|dist)[\\/]index\.(?:ts|js)$/.test( + !/[\\/]packages[\\/]core[\\/](?:src|dist[\\/]src)[\\/]index\.(?:ts|js)$/.test( args.importer, ) ) { diff --git a/packages/core/src/utils/iconvHelper.ts b/packages/core/src/utils/iconvHelper.ts index 7cb90cb32dc..d9118abab2c 100644 --- a/packages/core/src/utils/iconvHelper.ts +++ b/packages/core/src/utils/iconvHelper.ts @@ -23,8 +23,6 @@ interface IconvLite { import iconvModule from 'iconv-lite'; const iconvLite: IconvLite = iconvModule as unknown as IconvLite; -export { isUtf8CompatibleEncoding } from './encoding.js'; - /** * Decode a buffer using the specified encoding. * @param buffer The buffer to decode @@ -53,11 +51,3 @@ export function iconvEncode(content: string, encoding: string): Buffer { export function iconvEncodingExists(encoding: string): boolean { return iconvLite.encodingExists(encoding); } - -/** - * Check whether an encoding name represents a UTF-8 compatible encoding - * that Node's Buffer can handle natively without iconv-lite. - * Normalizes encoding names (e.g. 'utf-8', 'UTF8', 'us-ascii' all match). - * @param encoding The encoding name to check - * @returns True if the encoding is UTF-8 or ASCII compatible - */ diff --git a/packages/core/src/utils/load-iconv-lite.test.ts b/packages/core/src/utils/load-iconv-lite.test.ts index 706cf69bb11..7b00c35d011 100644 --- a/packages/core/src/utils/load-iconv-lite.test.ts +++ b/packages/core/src/utils/load-iconv-lite.test.ts @@ -27,4 +27,25 @@ describe('loadIconvLite', () => { expect(second).toBe(first); await expect(first).resolves.toBe(iconvLite); }); + + it('uses named exports', async () => { + const iconvLite = { + decode: vi.fn(), + encode: vi.fn(), + encodingExists: vi.fn(), + }; + vi.doMock('iconv-lite', () => iconvLite); + const { loadIconvLite } = await import('./load-iconv-lite.js'); + + await expect(loadIconvLite()).resolves.toEqual(iconvLite); + }); + + it('rejects an unexpected module shape', async () => { + vi.doMock('iconv-lite', () => ({ default: undefined })); + const { loadIconvLite } = await import('./load-iconv-lite.js'); + + await expect(loadIconvLite()).rejects.toThrow( + 'iconv-lite module does not match the expected API', + ); + }); }); diff --git a/packages/core/src/utils/load-iconv-lite.ts b/packages/core/src/utils/load-iconv-lite.ts index 69ba091bf77..e6c5f21de41 100644 --- a/packages/core/src/utils/load-iconv-lite.ts +++ b/packages/core/src/utils/load-iconv-lite.ts @@ -12,9 +12,28 @@ export interface IconvLite { let iconvLiteModulePromise: Promise | undefined; -export function loadIconvLite(): Promise { - iconvLiteModulePromise ??= import('iconv-lite').then( - (module) => module.default as unknown as IconvLite, +function isIconvLite(candidate: Partial): candidate is IconvLite { + return ( + 'decode' in candidate && + typeof candidate.decode === 'function' && + 'encode' in candidate && + typeof candidate.encode === 'function' && + 'encodingExists' in candidate && + typeof candidate.encodingExists === 'function' ); +} + +export function loadIconvLite(): Promise { + iconvLiteModulePromise ??= import('iconv-lite').then((module) => { + const imported = module as unknown as Partial & { + default?: IconvLite; + }; + const candidate = + 'default' in imported && imported.default ? imported.default : imported; + if (!isIconvLite(candidate)) { + throw new Error('iconv-lite module does not match the expected API'); + } + return candidate; + }); return iconvLiteModulePromise; } diff --git a/packages/core/src/utils/sync-file-encoding.test.ts b/packages/core/src/utils/sync-file-encoding.test.ts new file mode 100644 index 00000000000..f1e2b851669 --- /dev/null +++ b/packages/core/src/utils/sync-file-encoding.test.ts @@ -0,0 +1,50 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { describe, expect, it } from 'vitest'; +import { encodeTextFileContentAsync } from '../services/fileSystemService.js'; +import { decodeBufferWithEncodingInfoAsync } from './fileUtils.js'; +import { + decodeBufferWithEncodingInfo, + encodeTextFileContent, +} from './sync-file-encoding.js'; + +describe('sync file encoding compatibility', () => { + it.each([ + ['empty', Buffer.alloc(0)], + ['UTF-8', Buffer.from('Hello, 世界', 'utf8')], + [ + 'UTF-8 BOM', + Buffer.concat([ + Buffer.from([0xef, 0xbb, 0xbf]), + Buffer.from('Hello', 'utf8'), + ]), + ], + [ + 'GBK', + Buffer.from([ + 0xc4, 0xe3, 0xba, 0xc3, 0xca, 0xc0, 0xbd, 0xe7, 0xd5, 0xe2, 0xca, 0xc7, + 0xd6, 0xd0, 0xce, 0xc4, 0xb2, 0xe2, 0xca, 0xd4, + ]), + ], + ])('matches the async decoder for %s input', async (_name, input) => { + expect(await decodeBufferWithEncodingInfoAsync(input)).toEqual( + decodeBufferWithEncodingInfo(input), + ); + }); + + it.each([ + ['UTF-8', undefined], + ['CRLF', { lineEnding: 'crlf' as const }], + ['UTF-8 BOM', { bom: true }], + ['GBK', { encoding: 'gb18030' }], + ['unsupported encoding', { encoding: 'unsupported-codec' }], + ])('matches the async encoder for %s metadata', async (_name, meta) => { + expect( + await encodeTextFileContentAsync('/test/file.txt', 'Hello\n世界\n', meta), + ).toEqual(encodeTextFileContent('/test/file.txt', 'Hello\n世界\n', meta)); + }); +}); From fc99f597561bf82b6654ab2c124c1b094d00d8a2 Mon Sep 17 00:00:00 2001 From: doudouOUC Date: Sat, 25 Jul 2026 10:59:07 +0800 Subject: [PATCH 5/5] fix(core): Validate lazy dependency module shapes Co-authored-by: Qwen-Coder --- .../core/src/utils/load-simple-git.test.ts | 2 +- packages/core/src/utils/load-simple-git.ts | 24 +++++++++++++---- .../src/utils/load-xterm-headless.test.ts | 9 +++++++ .../core/src/utils/load-xterm-headless.ts | 27 +++++++++++++++---- 4 files changed, 51 insertions(+), 11 deletions(-) diff --git a/packages/core/src/utils/load-simple-git.test.ts b/packages/core/src/utils/load-simple-git.test.ts index eba9a92f4b3..6b547b83e72 100644 --- a/packages/core/src/utils/load-simple-git.test.ts +++ b/packages/core/src/utils/load-simple-git.test.ts @@ -47,7 +47,7 @@ describe('loadSimpleGit', () => { vi.doMock('simple-git', () => ({ CheckRepoActions: undefined, simpleGit: undefined, - default: undefined, + default: {}, })); const { loadSimpleGit } = await import('./load-simple-git.js'); diff --git a/packages/core/src/utils/load-simple-git.ts b/packages/core/src/utils/load-simple-git.ts index 40ac04e3263..6e209422776 100644 --- a/packages/core/src/utils/load-simple-git.ts +++ b/packages/core/src/utils/load-simple-git.ts @@ -13,16 +13,30 @@ export type SimpleGitModule = { let simpleGitModulePromise: Promise | undefined; +function isSimpleGitModule( + candidate: Partial | undefined, +): candidate is SimpleGitModule { + return ( + candidate !== undefined && + 'simpleGit' in candidate && + typeof candidate.simpleGit === 'function' && + 'CheckRepoActions' in candidate && + typeof candidate.CheckRepoActions === 'object' && + candidate.CheckRepoActions !== null + ); +} + export function loadSimpleGit(): Promise { simpleGitModulePromise ??= import('simple-git').then((module) => { const imported = module as unknown as Partial & { default?: SimpleGitModule; }; - const candidate = - imported.simpleGit && imported.CheckRepoActions - ? (imported as SimpleGitModule) - : imported.default; - if (!candidate) { + const candidate = isSimpleGitModule(imported) + ? imported + : 'default' in imported + ? imported.default + : undefined; + if (!isSimpleGitModule(candidate)) { throw new Error('simple-git module does not match the expected API'); } return { diff --git a/packages/core/src/utils/load-xterm-headless.test.ts b/packages/core/src/utils/load-xterm-headless.test.ts index a9886ea2c63..2be04be751e 100644 --- a/packages/core/src/utils/load-xterm-headless.test.ts +++ b/packages/core/src/utils/load-xterm-headless.test.ts @@ -31,4 +31,13 @@ describe('loadXtermHeadless', () => { await expect(loadXtermHeadless()).resolves.toEqual({ Terminal }); }); + + it('rejects an unexpected module shape', async () => { + vi.doMock('@xterm/headless', () => ({ default: {} })); + const { loadXtermHeadless } = await import('./load-xterm-headless.js'); + + await expect(loadXtermHeadless()).rejects.toThrow( + '@xterm/headless module does not match the expected API', + ); + }); }); diff --git a/packages/core/src/utils/load-xterm-headless.ts b/packages/core/src/utils/load-xterm-headless.ts index eb4704b3920..0992666fd2f 100644 --- a/packages/core/src/utils/load-xterm-headless.ts +++ b/packages/core/src/utils/load-xterm-headless.ts @@ -12,13 +12,30 @@ export type XtermHeadlessModule = { let xtermHeadlessModulePromise: Promise | undefined; +function isXtermHeadlessModule( + candidate: Partial | undefined, +): candidate is XtermHeadlessModule { + return ( + candidate !== undefined && + 'Terminal' in candidate && + typeof candidate.Terminal === 'function' + ); +} + export function loadXtermHeadless(): Promise { xtermHeadlessModulePromise ??= import('@xterm/headless').then((module) => { - const candidate = - 'Terminal' in module - ? module - : (module as unknown as { default: XtermHeadlessModule }).default; - return candidate as XtermHeadlessModule; + const imported = module as unknown as Partial & { + default?: XtermHeadlessModule; + }; + const candidate = isXtermHeadlessModule(imported) + ? imported + : 'default' in imported + ? imported.default + : undefined; + if (!isXtermHeadlessModule(candidate)) { + throw new Error('@xterm/headless module does not match the expected API'); + } + return candidate; }); return xtermHeadlessModulePromise; }