From 656846ed1b24828cc9448b69dec55603594378c7 Mon Sep 17 00:00:00 2001 From: labulalala <1565474717@qq.com> Date: Thu, 25 Jun 2026 16:04:54 +0800 Subject: [PATCH 1/3] =?UTF-8?q?fix:=20update=20Node=2020=20=E2=86=92=2024?= =?UTF-8?q?=20in=20docs/runtime=20+=20add=20install=20spinner?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - README (en/ja/zh), SETUP (en/zh): Node.js 20+ → 24+, >=20.0.0 → >=24.0.0 - F113 doc: fnm/Node.js 20 → 24, brew install node@20 → node@24 - desktop/service-manager.js: error message >=20 → >=24 - desktop/scripts/build-mac.sh: fallback version v22.12.0 → v24.16.0 - cli-spawn.ts comment: v20 → v24 - scripts/install.ps1: replace Tee-Object with live spinner for pnpm install progress, re-emit captured output on failure so real errors are visible to the user Co-Authored-By: Claude --- README.ja-JP.md | 2 +- README.md | 2 +- README.zh-CN.md | 2 +- SETUP.md | 2 +- SETUP.zh-CN.md | 2 +- desktop/scripts/build-mac.sh | 4 +- desktop/service-manager.js | 2 +- .../F113-multi-platform-one-click-deploy.md | 4 +- packages/api/src/utils/cli-spawn.ts | 2 +- scripts/install.ps1 | 119 ++++++++++++++++-- 10 files changed, 118 insertions(+), 23 deletions(-) diff --git a/README.ja-JP.md b/README.ja-JP.md index b85bb8c5d1..0e3b956d7d 100644 --- a/README.ja-JP.md +++ b/README.ja-JP.md @@ -85,7 +85,7 @@ Clowder はモデル非依存です。各エージェント CLI / adapter は統 ### オプション B：ソースセットアップ -**前提条件：** [Node.js 20+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *(オプション — スキップする場合は `--memory`)* · Git +**前提条件：** [Node.js 24+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *(オプション — スキップする場合は `--memory`)* · Git ```bash # 1. クローン diff --git a/README.md b/README.md index a0fa66b3a0..1a573ba357 100644 --- a/README.md +++ b/README.md @@ -85,7 +85,7 @@ The desktop installer bundles the app runtime, portable Node.js, and Redis, so r ### Option B: Source Setup -**Prerequisites:** [Node.js 20+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *(optional — use `--memory` to skip)* · Git +**Prerequisites:** [Node.js 24+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *(optional — use `--memory` to skip)* · Git ```bash # 1. Clone diff --git a/README.zh-CN.md b/README.zh-CN.md index 8cfd0ace66..03c6575197 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -79,7 +79,7 @@ Clowder 不绑定模型。当前支持的 Agent CLI / adapter： ### 方式 B：源码安装 -**前置要求：** [Node.js 20+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *（可选 — 用 `--memory` 跳过）* · Git +**前置要求：** [Node.js 24+](https://nodejs.org/) · [pnpm 9+](https://pnpm.io/) · [Redis 7+](https://redis.io/) *（可选 — 用 `--memory` 跳过）* · Git ```bash # 1. 克隆 diff --git a/SETUP.md b/SETUP.md index 013a2abf0e..876daa8003 100644 --- a/SETUP.md +++ b/SETUP.md @@ -8,7 +8,7 @@ | Tool | Version | Install | |------|---------|---------| -| **Node.js** | >= 20.0.0 | [nodejs.org](https://nodejs.org/) | +| **Node.js** | >= 24.0.0 | [nodejs.org](https://nodejs.org/) | | **pnpm** | >= 9.0.0 | `npm install -g pnpm` | | **Redis** | >= 7.0 | `brew install redis` (macOS) or [redis.io](https://redis.io/download/) — *optional: use `--memory` flag to skip* | | **Git** | any recent | Comes with most systems | diff --git a/SETUP.zh-CN.md b/SETUP.zh-CN.md index b366481fdd..81b62f2188 100644 --- a/SETUP.zh-CN.md +++ b/SETUP.zh-CN.md @@ -8,7 +8,7 @@ | 工具 | 版本 | 安装方式 | |------|------|---------| -| **Node.js** | >= 20.0.0 | [nodejs.org](https://nodejs.org/) | +| **Node.js** | >= 24.0.0 | [nodejs.org](https://nodejs.org/) | | **pnpm** | >= 9.0.0 | `npm install -g pnpm` | | **Redis** | >= 7.0 | `brew install redis`（macOS）或 [redis.io](https://redis.io/download/) — *可选：用 `--memory` 标志跳过* | | **Git** | 任意近期版本 | 大多数系统自带 | diff --git a/desktop/scripts/build-mac.sh b/desktop/scripts/build-mac.sh index 8f8f81de2e..d47a26202e 100755 --- a/desktop/scripts/build-mac.sh +++ b/desktop/scripts/build-mac.sh @@ -111,8 +111,8 @@ bold "Step 3/6 — Bundle Node.js portable (arm64 + x64)" # matches the bundled runtime. Same rationale as the Windows build. BUILD_NODE_VERSION="$(node --version 2>/dev/null || echo '')" if [[ -z "$BUILD_NODE_VERSION" ]]; then - warn "node not on PATH; defaulting to v22.12.0" - BUILD_NODE_VERSION="v22.12.0" + warn "node not on PATH; defaulting to v24.16.0" + BUILD_NODE_VERSION="v24.16.0" fi BUILD_NODE_MAJOR="${BUILD_NODE_VERSION#v}" BUILD_NODE_MAJOR="${BUILD_NODE_MAJOR%%.*}" diff --git a/desktop/service-manager.js b/desktop/service-manager.js index 4637b1c40d..f1ca111f74 100644 --- a/desktop/service-manager.js +++ b/desktop/service-manager.js @@ -100,7 +100,7 @@ class ServiceManager { // ---- Pre-flight checks ---- const nodeExe = resolveNode(this.root); if (!nodeExe) { - throw new Error('Node.js not found. Please install Node.js >= 20 from https://nodejs.org/'); + throw new Error('Node.js not found. Please install Node.js >= 24 from https://nodejs.org/'); } log(`Node.js found: ${nodeExe}`); diff --git a/docs/features/F113-multi-platform-one-click-deploy.md b/docs/features/F113-multi-platform-one-click-deploy.md index 3f28deaaac..e32c096fee 100644 --- a/docs/features/F113-multi-platform-one-click-deploy.md +++ b/docs/features/F113-multi-platform-one-click-deploy.md @@ -26,7 +26,7 @@ community_issue: https://github.com/zts212653/clowder-ai/issues/14 `scripts/install.sh` — 自动检测 Debian/Ubuntu、CentOS/RHEL/Fedora，安装依赖、配置环境变量、构建并启动服务。 - 系统依赖检测 + 自动安装（git、curl） -- fnm/Node.js 20 + pnpm + Redis +- fnm/Node.js 24 + pnpm + Redis - AI CLI 工具安装（Claude Code、Codex、Gemini CLI） - TTY 安全读取（非交互环境兼容） - npm registry fallback（中国镜像） @@ -41,7 +41,7 @@ community_issue: https://github.com/zts212653/clowder-ai/issues/14 核心能力： - **Homebrew 自动安装**：Apple Silicon（`/opt/homebrew`）+ Intel（`/usr/local`）双路径检测；非 login shell 下也能发现已有 brew（`shellenv` 注入） - **Xcode CLT**：缺失时自动 `xcode-select --install`，30min 非致命超时（不阻塞安装） -- **Node.js**：优先 fnm，fallback `brew install node@20`；keg-only formula 的 bin 路径显式注入 PATH +- **Node.js**：优先 fnm，fallback `brew install node@24`；keg-only formula 的 bin 路径显式注入 PATH - **pnpm**：corepack → npm fallback；安装后无条件 `persist_user_bin` + 当前 session PATH 注入 - **Redis**：`brew install redis` + `brew services start redis`（启动失败非致命，install 是关键） - **AI CLI 工具**： diff --git a/packages/api/src/utils/cli-spawn.ts b/packages/api/src/utils/cli-spawn.ts index 687a3ca8c4..a26362e5c3 100644 --- a/packages/api/src/utils/cli-spawn.ts +++ b/packages/api/src/utils/cli-spawn.ts @@ -959,7 +959,7 @@ function defaultSpawn( // managers (nvm/fnm/Volta). CLI shims use `#!/usr/bin/env node`, so the // child process must be able to find `node` in its PATH. Prepend the // directory containing the resolved CLI binary — it typically sits next - // to the `node` binary that installed it (e.g. ~/.nvm/versions/node/v20/bin/). + // to the `node` binary that installed it (e.g. ~/.nvm/versions/node/v24/bin/). const env = { ...options.env }; if (isAbsolute(command)) { const binDir = dirname(command); diff --git a/scripts/install.ps1 b/scripts/install.ps1 index 47274397a5..d091400c05 100644 --- a/scripts/install.ps1 +++ b/scripts/install.ps1 @@ -34,10 +34,6 @@ function Refresh-Path { function Resolve-PnpmCommand { Resolve-ToolCommand -Name "pnpm" } function Invoke-Pnpm { param([string[]]$CommandArgs) Invoke-ToolCommand -Name "pnpm" -CommandArgs $CommandArgs } -function Get-CommandOutputText { - param([object[]]$OutputLines) - return (@($OutputLines) | ForEach-Object { "$_" }) -join "`n" -} function Test-PuppeteerBrowserDownloadFailure { param([string]$OutputText) return $OutputText -match "puppeteer" -and @@ -90,7 +86,7 @@ function Invoke-PnpmInstallWithCapturedOutput { [switch]$SkipPuppeteerDownload ) - $capturedOutput = @() + $capturedOutput = [System.Collections.Generic.List[string]]::new() $hadPreviousSkip = Test-Path Env:PUPPETEER_SKIP_DOWNLOAD $previousSkipValue = if ($hadPreviousSkip) { $env:PUPPETEER_SKIP_DOWNLOAD } else { $null } $previousErrorActionPreference = $ErrorActionPreference @@ -110,6 +106,23 @@ function Invoke-PnpmInstallWithCapturedOutput { } } + # Spinner state for live progress display. + # Use a hashtable so ForEach-Object scriptblock mutations are visible + # in the outer scope (PowerShell 5.1 closes over reference types but + # not value types — $script:scope would also work, but a hashtable + # is cleaner inside a function that may be dot-sourced). + $spin = @{ + Chars = @('-', '\', '|', '/') + Index = 0 + Last = "" + Line = "" + CanUse = $false + } + try { + [void][Console]::CursorVisible + $spin.CanUse = $true + } catch {} + try { if ($SkipPuppeteerDownload) { $env:PUPPETEER_SKIP_DOWNLOAD = "1" @@ -134,16 +147,77 @@ function Invoke-PnpmInstallWithCapturedOutput { # Stop, PowerShell 5.1 can promote benign Node 24 stderr (DEP0169) # into a RemoteException before we can read pnpm's exit code. $ErrorActionPreference = "SilentlyContinue" - & $pnpmCommand @CommandArgs 2>&1 | Tee-Object -Variable capturedOutput + + # Instead of Tee-Object, process each line through a custom handler + # that captures output AND shows a live spinner for progress lines. + # Non-progress lines (errors, warnings, etc.) are printed normally + # so they remain visible on screen. + & $pnpmCommand @CommandArgs 2>&1 | ForEach-Object { + $line = "$_" + $capturedOutput.Add($line) + + # Classify the line: is it a progress line that the spinner + # should display, or a real output line to print normally? + $isProgressLine = $line -match '^\.\.\./' -or + $line -match '^packages/' -or + $line -match '(?:Progress|Already|Downloaded|Resolved|Reused):' + + if ($spin.CanUse -and $isProgressLine) { + # Update spinner with progress info + if ($line -match '^\.\.\./(.+?)(?:\s+|\$)') { + $spin.Last = $Matches[1] + } elseif ($line -match '^packages/(.+?)(?:\s|$)') { + $spin.Last = $Matches[1] + } elseif ($line -match '(?:Progress|Already|Downloaded|Resolved|Reused):\s*(.+)') { + $spin.Last = $Matches[1].Trim() + } + + $ch = $spin.Chars[$spin.Index % $spin.Chars.Count] + $spin.Index++ + $display = if ($spin.Last) { $spin.Last } else { "resolving..." } + if ($display.Length -gt 50) { $display = $display.Substring(0, 47) + "..." } + $spin.Line = " $ch $display" + try { + [Console]::SetCursorPosition(0, [Console]::CursorTop) + [Console]::Write("`r$($spin.Line)" + " " * ([Math]::Max(0, 60 - $spin.Line.Length))) + } catch {} + } else { + # Non-progress line: clear spinner, print on its own line + if ($spin.CanUse -and $spin.Line) { + try { + [Console]::SetCursorPosition(0, [Console]::CursorTop) + [Console]::Write((" " * 70) + "`r") + } catch {} + $spin.Line = "" + } + Write-Host " $line" + } + } + + # Clear the progress line after pnpm finishes + if ($spin.CanUse -and $spin.Line) { + try { + [Console]::SetCursorPosition(0, [Console]::CursorTop) + [Console]::Write((" " * 70) + "`r") + } catch {} + } + return [pscustomobject]@{ Ok = $global:LASTEXITCODE -eq 0 ErrorRecord = $null - OutputText = Get-CommandOutputText -OutputLines $capturedOutput + OutputText = $capturedOutput -join "`n" } } catch { + # Clear progress line on error too + if ($spin.CanUse -and $spin.Line) { + try { + [Console]::SetCursorPosition(0, [Console]::CursorTop) + [Console]::Write((" " * 70) + "`r") + } catch {} + } # Two distinct scenarios reach this catch: - # (a) pnpm actually ran, exited 0, and only the 2>&1 | Tee-Object - # pipeline threw (e.g. Node 24 DEP0169 deprecation on stderr + # (a) pnpm actually ran, exited 0, and only the 2>&1 pipeline + # threw (e.g. Node 24 DEP0169 deprecation on stderr # under $ErrorActionPreference=Stop). $global:LASTEXITCODE is # now 0 and we should treat this as success. # (b) pnpm itself failed before producing an exit code, or the @@ -154,13 +228,13 @@ function Invoke-PnpmInstallWithCapturedOutput { return [pscustomobject]@{ Ok = $true ErrorRecord = $null - OutputText = Get-CommandOutputText -OutputLines ($capturedOutput + @($_)) + OutputText = ($capturedOutput + @("$_")) -join "`n" } } return [pscustomobject]@{ Ok = $false ErrorRecord = $_ - OutputText = Get-CommandOutputText -OutputLines ($capturedOutput + @($_)) + OutputText = ($capturedOutput + @("$_")) -join "`n" } } } finally { @@ -494,6 +568,16 @@ if (-not $frozenInstallResult.Ok) { } if (-not $plainInstallResult.Ok) { Exit-InstallerIfCancelled -ErrorRecord $plainInstallResult.ErrorRecord -Context "pnpm install" + # Re-emit captured pnpm output so the user can see the real error + if ($plainInstallResult.OutputText) { + Write-Host "" + Write-Host " --- pnpm output ---" -ForegroundColor DarkGray + $plainInstallResult.OutputText -split "`n" | ForEach-Object { + Write-Host " $_" -ForegroundColor DarkGray + } + Write-Host " --- end pnpm output ---" -ForegroundColor DarkGray + Write-Host "" + } Write-Err "pnpm install failed" exit 1 } @@ -504,8 +588,19 @@ if (-not $frozenInstallResult.Ok) { if (Test-WindowsEpermFailure -OutputText $frozenInstallResult.OutputText) { Write-WindowsEpermHint } + # The spinner suppresses pnpm output during the run. Re-emit it now + # so the user can see the actual error pnpm reported. + if ($frozenInstallResult.OutputText) { + Write-Host "" + Write-Host " --- pnpm output ---" -ForegroundColor DarkGray + $frozenInstallResult.OutputText -split "`n" | ForEach-Object { + Write-Host " $_" -ForegroundColor DarkGray + } + Write-Host " --- end pnpm output ---" -ForegroundColor DarkGray + Write-Host "" + } Write-Err "pnpm install --frozen-lockfile failed" - Write-Err "The real error is above. This is NOT a lockfile drift issue." + Write-Err "See pnpm output above for the real error. This is NOT a lockfile drift issue." exit 1 } } From e4044be347765a8d9081f9f81142592c28263ff1 Mon Sep 17 00:00:00 2001 From: labulalala <1565474717@qq.com> Date: Thu, 25 Jun 2026 16:42:29 +0800 Subject: [PATCH 2/3] fix(test): update install-script-error-classification tests for ForEach-Object spinner The pnpm install pipeline changed from Tee-Object to ForEach-Object for live spinner progress display. Update test assertions to accept either pipeline type while preserving the core invariants: - pnpm must be invoked directly (not via Invoke-Pnpm wrapper) - $global:LASTEXITCODE must be read/written explicitly - ErrorActionPreference must be downgraded around pnpm capture Also fix catch-block extraction: the function now has nested bare catch {} blocks for spinner operations which broke greedy regex matching. Use simpler positional checks instead. Co-Authored-By: Claude --- ...nstall-script-error-classification.test.js | 50 +++++++++++++------ 1 file changed, 34 insertions(+), 16 deletions(-) diff --git a/packages/api/test/install-script-error-classification.test.js b/packages/api/test/install-script-error-classification.test.js index f77bfb7dcb..cb22c692c6 100644 --- a/packages/api/test/install-script-error-classification.test.js +++ b/packages/api/test/install-script-error-classification.test.js @@ -156,15 +156,27 @@ test('Step 5 fails fast on non-lockfile errors instead of swapping to plain pnpm test('Invoke-PnpmInstallWithCapturedOutput trusts $LASTEXITCODE over pipeline exceptions (DEP0169 tolerance)', () => { // Node 24 emits DEP0169 deprecation warnings to stderr. With $ErrorActionPreference=Stop, - // the 2>&1 | Tee-Object pipeline can throw even when pnpm itself exited 0. + // the 2>&1 pipeline can throw even when pnpm itself exited 0. // The catch path must check $LASTEXITCODE and treat exit 0 as success, not failure. const body = getInvokePnpmInstallFunctionBody(); - // The catch block must reference $LASTEXITCODE so it can distinguish a real - // process failure from a benign pipeline throw on stderr. - const catchBlock = body.match(/} catch \{[\s\S]*?\}/); - assert.ok(catchBlock, 'must have catch block'); + // The function has a catch block that checks $LASTEXITCODE after the pipeline + // throws. Rather than trying to extract the catch block with regex (fragile + // due to nested bare catch {} blocks for spinner operations), verify the + // invariant: a multi-line catch block exists that contains the LASTEXITCODE + // check. The multi-line catch is '} catch {' followed by a newline (not the + // bare '} catch {}' one-liners). + const multiLineCatchIdx = body.indexOf('} catch {\r\n'); + if (multiLineCatchIdx < 0) { + // Also try LF-only + const altIdx = body.indexOf('} catch {\n'); + assert.ok(altIdx >= 0, 'must have multi-line catch block'); + } + // The LASTEXITCODE check must appear after the catch keyword + const catchIdx = body.indexOf('} catch {'); + assert.ok(catchIdx >= 0, 'must have catch block'); + const afterCatch = body.substring(catchIdx); assert.match( - catchBlock[0], + afterCatch, /\$(?:global:)?LASTEXITCODE\s*-eq\s*0/, 'catch block must check $LASTEXITCODE (optionally with $global: scope) -eq 0 to avoid DEP0169 false failures', ); @@ -204,22 +216,23 @@ test('Invoke-PnpmInstallWithCapturedOutput calls resolved pnpm directly inside t // (Invoke-Pnpm -> Invoke-ToolCommand -> & $toolCommand) hides the native exit // code from the captured pipeline. Fix: resolve pnpm up-front and invoke it // directly inside the captured pipeline (`& $pnpmCommand @CommandArgs 2>&1 - // | Tee-Object ...`) so the native command is the only producer of + // | ForEach-Object ...`) so the native command is the only producer of // $LASTEXITCODE in scope. const body = getInvokePnpmInstallFunctionBody(); assert.match(body, /Resolve-PnpmCommand/, 'must resolve pnpm command upfront (not via Invoke-Pnpm wrapper)'); // The captured pipeline must invoke the resolved pnpm command directly with // `& $pnpmCommand`, NOT through the Invoke-Pnpm wrapper, so PowerShell can - // record the native exit code in $LASTEXITCODE reliably. + // record the native exit code in $LASTEXITCODE reliably. The pipeline may + // use either Tee-Object or ForEach-Object — both preserve $LASTEXITCODE. assert.match( body, - /&\s*\$pnpmCommand\s+@CommandArgs\s+2>&1\s*\|\s*Tee-Object/, - 'captured pipeline must call & $pnpmCommand @CommandArgs 2>&1 | Tee-Object directly', + /&\s*\$pnpmCommand\s+@CommandArgs\s+2>&1\s*\|\s*(?:Tee-Object|ForEach-Object)/, + 'captured pipeline must call & $pnpmCommand @CommandArgs 2>&1 | Tee-Object or ForEach-Object directly', ); assert.doesNotMatch( body, - /Invoke-Pnpm\s+-CommandArgs\s+\$CommandArgs\s+2>&1\s*\|\s*Tee-Object/, - 'must NOT pipe Invoke-Pnpm into Tee-Object (hides native exit code from caller scope)', + /Invoke-Pnpm\s+-CommandArgs\s+\$CommandArgs\s+2>&1\s*\|\s*(?:Tee-Object|ForEach-Object)/, + 'must NOT pipe Invoke-Pnpm into Tee-Object/ForEach-Object (hides native exit code from caller scope)', ); }); @@ -247,10 +260,15 @@ test('Invoke-PnpmInstallWithCapturedOutput reads $global:LASTEXITCODE explicitly assert.ok(successCheck, 'must have an Ok = $... -eq 0 check on the success path'); assert.equal(successCheck[1], '$global:LASTEXITCODE', 'success path must read $global:LASTEXITCODE explicitly'); // The catch path must also read $global:LASTEXITCODE for the same reason. - const catchBlock = body.match(/} catch \{[\s\S]*?\}\s*\}\s*finally/); - assert.ok(catchBlock, 'must have catch block'); + // Rather than extracting the catch block with regex (fragile due to nested + // bare catch {} blocks for spinner operations), verify the invariant: + // $global:LASTEXITCODE appears in a catch context (after '} catch {'). + const catchIdx = body.indexOf('} catch {'); + assert.ok(catchIdx >= 0, 'must have catch block'); + const afterCatch = body.substring(catchIdx); assert.match( - catchBlock[0], + afterCatch, + /\$global:LASTEXITCODE\s*-eq\s*0/, /\$global:LASTEXITCODE\s*-eq\s*0/, 'catch path must read $global:LASTEXITCODE explicitly to detect pnpm success-with-pipeline-throw', ); @@ -260,7 +278,7 @@ test('Invoke-PnpmInstallWithCapturedOutput temporarily downgrades ErrorActionPre const body = getInvokePnpmInstallFunctionBody(); const snapshotIdx = body.indexOf('$previousErrorActionPreference = $ErrorActionPreference'); const downgradeIdx = body.indexOf('$ErrorActionPreference = "SilentlyContinue"'); - const invokeMatch = body.match(/&\s*\$pnpmCommand\s+@CommandArgs\s+2>&1\s*\|\s*Tee-Object/); + const invokeMatch = body.match(/&\s*\$pnpmCommand\s+@CommandArgs\s+2>&1\s*\|\s*(?:Tee-Object|ForEach-Object)/); const invokeIdx = invokeMatch ? invokeMatch.index : -1; const finallyIdx = body.indexOf('} finally {'); const restoreIdx = body.indexOf('$ErrorActionPreference = $previousErrorActionPreference'); From c28e9b9f30c3c8eba750fc507ec004922d4a4115 Mon Sep 17 00:00:00 2001 From: labulalala <1565474717@qq.com> Date: Thu, 25 Jun 2026 17:33:23 +0800 Subject: [PATCH 3/3] fix(test): anchor catch-block invariants to DEP0169 comment marker MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewer feedback: the previous test used body.indexOf('} catch {') which matched the spinner capability probe (bare catch {}) instead of the actual pnpm pipeline catch block. A future edit could remove $global:LASTEXITCODE from the real catch and the tests would still pass — invariant regression. Fix: anchor both DEP0169 tolerance tests to the "Two distinct scenarios reach this catch:" comment, then extract the catch block between that and the following '} finally {'. This precisely targets the pnpm pipeline catch path. Also fix duplicated regex argument in the second assert.match call (the intended message string was being ignored). Verified: removing $global:LASTEXITCODE -eq 0 from the catch block causes both tests to fail as expected. Co-Authored-By: Claude --- ...nstall-script-error-classification.test.js | 53 ++++++++++--------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/packages/api/test/install-script-error-classification.test.js b/packages/api/test/install-script-error-classification.test.js index cb22c692c6..fa380fba96 100644 --- a/packages/api/test/install-script-error-classification.test.js +++ b/packages/api/test/install-script-error-classification.test.js @@ -159,24 +159,23 @@ test('Invoke-PnpmInstallWithCapturedOutput trusts $LASTEXITCODE over pipeline ex // the 2>&1 pipeline can throw even when pnpm itself exited 0. // The catch path must check $LASTEXITCODE and treat exit 0 as success, not failure. const body = getInvokePnpmInstallFunctionBody(); - // The function has a catch block that checks $LASTEXITCODE after the pipeline - // throws. Rather than trying to extract the catch block with regex (fragile - // due to nested bare catch {} blocks for spinner operations), verify the - // invariant: a multi-line catch block exists that contains the LASTEXITCODE - // check. The multi-line catch is '} catch {' followed by a newline (not the - // bare '} catch {}' one-liners). - const multiLineCatchIdx = body.indexOf('} catch {\r\n'); - if (multiLineCatchIdx < 0) { - // Also try LF-only - const altIdx = body.indexOf('} catch {\n'); - assert.ok(altIdx >= 0, 'must have multi-line catch block'); - } - // The LASTEXITCODE check must appear after the catch keyword - const catchIdx = body.indexOf('} catch {'); - assert.ok(catchIdx >= 0, 'must have catch block'); - const afterCatch = body.substring(catchIdx); + // Anchor to the actual pnpm pipeline catch block by locating the + // "Two distinct scenarios reach this catch:" comment, which uniquely + // identifies the DEP0169 tolerance catch (not the bare spinner catch {}). + const scenariosIdx = body.indexOf('Two distinct scenarios'); + assert.ok(scenariosIdx >= 0, 'must have DEP0169 tolerance catch block with "Two distinct scenarios" comment'); + // Walk backward from the comment to find its enclosing '} catch {' + const beforeComment = body.substring(0, scenariosIdx); + const catchIdx = beforeComment.lastIndexOf('} catch {'); + assert.ok(catchIdx >= 0, 'must have catch block before "Two distinct scenarios" comment'); + // Slice from catch start to the following '} finally {' to capture the + // full pnpm pipeline catch block (excluding later code outside it). + const fromCatch = body.substring(catchIdx); + const finallyIdx = fromCatch.indexOf('} finally {'); + assert.ok(finallyIdx >= 0, 'catch block must be followed by } finally {'); + const catchBlock = fromCatch.substring(0, finallyIdx); assert.match( - afterCatch, + catchBlock, /\$(?:global:)?LASTEXITCODE\s*-eq\s*0/, 'catch block must check $LASTEXITCODE (optionally with $global: scope) -eq 0 to avoid DEP0169 false failures', ); @@ -260,15 +259,19 @@ test('Invoke-PnpmInstallWithCapturedOutput reads $global:LASTEXITCODE explicitly assert.ok(successCheck, 'must have an Ok = $... -eq 0 check on the success path'); assert.equal(successCheck[1], '$global:LASTEXITCODE', 'success path must read $global:LASTEXITCODE explicitly'); // The catch path must also read $global:LASTEXITCODE for the same reason. - // Rather than extracting the catch block with regex (fragile due to nested - // bare catch {} blocks for spinner operations), verify the invariant: - // $global:LASTEXITCODE appears in a catch context (after '} catch {'). - const catchIdx = body.indexOf('} catch {'); - assert.ok(catchIdx >= 0, 'must have catch block'); - const afterCatch = body.substring(catchIdx); + // Anchor to the pnpm pipeline catch block using the "Two distinct scenarios" + // comment, then extract the block between the catch and its following finally. + const scenariosIdx = body.indexOf('Two distinct scenarios'); + assert.ok(scenariosIdx >= 0, 'must have DEP0169 tolerance catch block with "Two distinct scenarios" comment'); + const beforeComment = body.substring(0, scenariosIdx); + const catchIdx = beforeComment.lastIndexOf('} catch {'); + assert.ok(catchIdx >= 0, 'must have catch block before "Two distinct scenarios" comment'); + const fromCatch = body.substring(catchIdx); + const finallyIdx = fromCatch.indexOf('} finally {'); + assert.ok(finallyIdx >= 0, 'catch block must be followed by } finally {'); + const catchBlock = fromCatch.substring(0, finallyIdx); assert.match( - afterCatch, - /\$global:LASTEXITCODE\s*-eq\s*0/, + catchBlock, /\$global:LASTEXITCODE\s*-eq\s*0/, 'catch path must read $global:LASTEXITCODE explicitly to detect pnpm success-with-pipeline-throw', );