fix(cli): classify nested update-check network errors

[yiliang114]

Note

Post-merge E2E testing corrected part of the original scope. The direct error.cause fix is reproducible through the real startup TUI. The claimed global-npm timeout differential is not: both the #7409 baseline and this PR already render registry did not respond within 5s because the outer typed timeout surfaces first. #7431 removes the unsupported child-process special case and restores the prior ETIMEDOUT classification.

What this PR does

This fixes the reproducible Node 22 fetch error shape behind #7423. Network error codes can live on a direct error.cause, so the update-check classifier now examines that cause as well as the top-level error.

Why it's needed

Node's fetch API reports DNS failures as a top-level TypeError: fetch failed while keeping ENOTFOUND on error.cause.code. The previous classifier inspected only the top-level code and message, so a real startup background check fell back to registry error instead of registry unreachable.

The merged revision also included child-process timeout and ETIMEDOUT classification changes. Complete global-npm testing after merge did not reproduce the claimed behavior difference, so those two changes are removed by #7431 rather than retained as speculative handling.

Reviewer Test Plan

How to verify

• Run cd packages/cli && npx vitest run src/ui/utils/updateCheck.test.ts.
• Build and bundle the CLI, then start the interactive TUI with an isolated Qwen home and npm_config_registry=http://this-host-does-not-exist-zzz.invalid; the background warning should report registry unreachable instead of registry error.

Evidence (Before & After)

Corrected real-startup tmux evidence and the global-npm timeout control are included in the E2E report comment below.

Tested on

OS	Status
🍏 macOS	✅
🪟 Windows	⚠️
🐧 Linux	⚠️

Environment (optional)

macOS 15.1.1, Node.js v22.22.0, tmux 3.5a. Windows and Linux are left to CI.

Risk & Scope

• Main risk or tradeoff: only the direct cause is inspected; recursive and aggregate cause traversal remain out of scope.
• Not validated / out of scope: manual Windows/Linux TUI verification.
• Breaking changes / migration notes: none.

Linked Issues

Fixes #7423

Related to #7409

Follow-up correction: #7431

中文说明

本 PR 可复现的修复是读取 Node 22 fetch 错误的直接 error.cause。真实启动 TUI 中，DNS 失败从 #7409 baseline 的 registry error 正确变为 registry unreachable。

合并后完成的 global-npm E2E 纠正了原描述中的 timeout 结论：baseline 与本 PR 都已经显示 registry did not respond within 5s，因为外层类型化超时会先返回。#7431 删除不受完整路径支持的子进程 timeout 特判，并恢复原有的 ETIMEDOUT 分类。

正确的启动 Before/After 图片及 timeout 无差异控制见下方独立测试评论。

[wenshao]

Code Review — fix(cli): classify real update-check failure shapes

Thanks for this. It's a tight, well-scoped fix and the reasoning in the PR body matches the real Node 22 error shapes. I traced the full flow (runGlobalNpm → execFile with only timeout: FETCH_TIMEOUT_MS, so the default killSignal = SIGTERM; the updateNotifier fetch path → TypeError: fetch failed carrying cause.code), and the classifier changes line up with both. Correctness looks solid — this is essentially good to merge, with a few minor/optional notes below.

What it does

• Reads network error codes from a direct error.cause, so a Node-fetch DNS failure (TypeError: fetch failed + cause.code = ENOTFOUND) is now classified offline instead of the generic registry.
• Detects timed-out execFile children (killed: true, signal: 'SIGTERM', code: null) and reports timeout.
• Moves ETIMEDOUT out of NETWORK_ERROR_CODES and into the timeout bucket.
• Adds regression tests for all three shapes.

Strengths

• The execFile-timeout signature (killed/SIGTERM) is exactly what Node produces for the timeout option, and runGlobalNpm doesn't override killSignal, so the heuristic is correct for the current call site.
• Timeout is checked before offline, so the reclassified ETIMEDOUT can't be shadowed by the offline branch.
• Tests use the observed shapes (new TypeError('fetch failed', { cause }), code: null / killed / SIGTERM) rather than synthetic ones — good regression value.

Minor / optional

• Variable shadowing (readability). The inner arrows reuse the name error, shadowing the outer parameter — e.g. errors.some((error) => error instanceof UpdateCheckTimeoutError) and inside matchesCode. It typechecks and doesn't trip lint (no-shadow isn't enabled here), but renaming the inner binding to e / candidate would read more clearly.
• Implicit coupling to the default kill signal. The signal === 'SIGTERM' check silently depends on runGlobalNpm's execFile not setting killSignal. If someone later sets killSignal: 'SIGKILL' (or an AbortSignal), timeout detection breaks with no test catching it. A one-line comment tying the two together, or asserting the execFile options in a test, would guard against that drift.
• ETIMEDOUT UX nuance. ETIMEDOUT is often a TCP connect timeout (host unreachable / firewall dropping SYNs), which is arguably closer to "unreachable" than "registry too slow". The reclassification flips the user-facing message from registry unreachable → registry did not respond within Ns. Defensible either way — just flagging that the wording now leans "slow registry" for a case that can also be "no route to host".
• Test coverage gaps (optional). Consider a negative case (killed: true with a non-SIGTERM signal, e.g. SIGKILL, should not be timeout) and a timeout code carried on error.cause — both exercise branches the current tests don't pin down.

Known limitation (already scoped out, noting for the record)

Only a single-level error.cause is traversed. Node/undici can surface multi-address connection failures as an AggregateError whose per-address codes live in .errors[]; if the top-level object doesn't also carry .code/the code in its message, such a real "offline" case still falls through to registry. The PR explicitly lists recursive/aggregate traversal as out of scope, so no change requested — just worth keeping in mind for the next iteration.

Risk

Low. The only behavioral change is the failure reason/message shown to the user; no control-flow or update-decision logic is touched. Windows/Linux execFile timeout shapes are left to CI, which is a reasonable call given SIGTERM semantics differ on Windows.

---

🤖 Generated with Claude Code — Claude Opus 4.8 (1M context)

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

Problem: observed bug with clear evidence. Issue #7423 documents the exact Node 22 error shapes (fetch DNS failure with ENOTFOUND on error.cause.code, execFile timeout with killed: true / signal: 'SIGTERM') and includes a reproduction. The before/after in the PR body confirms the misclassification.

Direction: aligned — this fixes user-facing error messages for update checks, directly in scope. No CHANGELOG reference needed for a classifier bugfix.

Size: not applicable (no core paths touched).

Approach: minimal and focused. Two files, one production change (~20 lines) and matching test updates. Every edit maps to a specific misclassification path from the issue. Nothing to cut.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

问题：已观测到的 bug，证据充分。Issue #7423 记录了 Node 22 的真实错误形态（fetch DNS 失败时 ENOTFOUND 在 error.cause.code 上，execFile 超时时 killed: true / signal: 'SIGTERM'），并附有复现代码。PR 正文的 before/after 确认了分类错误。

方向：对齐——修复更新检查的用户可见错误信息，完全在范围内。

规模：不适用（未触及核心路径）。

方案：最小且聚焦。两个文件，一处生产代码改动（约 20 行）加对应测试更新。每处改动都对应 issue 中的一个具体分类错误路径。无需删减。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

_{Reviewed at 5c76c37f4692454258e971cb95aeb643937b09e6 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Code Review

Independent proposal: given the two misclassification paths (fetch DNS failure hiding ENOTFOUND in error.cause, execFile timeout surfacing as killed/SIGTERM without a matching code), I would: (1) unwrap one level of error.cause when checking network codes, (2) detect killed === true && signal === 'SIGTERM' as timeout, (3) move ETIMEDOUT from the offline bucket to timeout since a socket-connect timeout reads as "did not respond" rather than "unreachable".

Comparison with the diff: the PR does exactly this. The errors array collects the top-level error and its direct cause; matchesCode checks .code and .message across both. Timeout detection covers UpdateCheckTimeoutError (moved inside the instanceof Error block — fine since it extends Error), the killed/signal shape, and ETIMEDOUT. The remaining network codes stay as offline. No simpler path exists.

No correctness issues, no security concerns, no convention violations. The variable shadowing in the some callback (error shadows the outer error) is idiomatic TypeScript and not worth flagging.

Unit tests: 41/41 pass, covering the execFile timeout shape, ETIMEDOUT → timeout, and cause-unwrapping for the fetch path.

Real-Scenario Testing

Built and bundled the CLI, then ran the update command against a non-existent registry host.

Before (main build)

$ npm_config_registry=http://this-host-does-not-exist-zzz.invalid node dist/cli.js update 2>&1; echo EXIT_CODE=$?
Failed to check for updates (registry error). Please check your network or registry configuration.
EXIT_CODE=1

After (this PR)

$ npm_config_registry=http://this-host-does-not-exist-zzz.invalid node dist/cli.js update 2>&1; echo EXIT_CODE=$?
Failed to check for updates (registry unreachable). Please check your network or registry configuration.
EXIT_CODE=1

DNS failure now correctly reports "registry unreachable" instead of the generic "registry error". Matches the PR's stated before/after.

中文说明

代码审查

独立方案： 针对两条分类错误路径（fetch DNS 失败将 ENOTFOUND 藏在 error.cause 中，execFile 超时以 killed/SIGTERM 形态出现但无匹配错误码），我会：(1) 检查网络错误码时解包一层 error.cause，(2) 将 killed === true && signal === 'SIGTERM' 识别为超时，(3) 将 ETIMEDOUT 从 offline 桶移至 timeout（socket 连接超时语义上更接近"未响应"而非"不可达"）。

与 diff 对比： PR 完全按此实现。errors 数组收集顶层错误及其直接 cause；matchesCode 在两者上检查 .code 和 .message。超时检测覆盖 UpdateCheckTimeoutError（移入 instanceof Error 块内——因为它继承自 Error，所以没问题）、killed/signal 形态和 ETIMEDOUT。其余网络错误码保持 offline。没有更简路径。

无正确性问题、无安全隐患、无规范违反。some 回调中的变量遮蔽（error 遮蔽外层 error）是惯用 TypeScript 写法，不值得标记。

单元测试：41/41 通过，覆盖 execFile 超时形态、ETIMEDOUT → timeout、以及 fetch 路径的 cause 解包。

真实场景测试

构建并 bundle CLI 后，对不存在的 registry 主机运行 update 命令。

Before（main 构建）

$ npm_config_registry=http://this-host-does-not-exist-zzz.invalid node dist/cli.js update 2>&1; echo EXIT_CODE=$?
Failed to check for updates (registry error). Please check your network or registry configuration.
EXIT_CODE=1

After（本 PR）

$ npm_config_registry=http://this-host-does-not-exist-zzz.invalid node dist/cli.js update 2>&1; echo EXIT_CODE=$?
Failed to check for updates (registry unreachable). Please check your network or registry configuration.
EXIT_CODE=1

DNS 失败现在正确报告 "registry unreachable" 而非通用的 "registry error"。与 PR 描述的 before/after 一致。

— Qwen Code · qwen3.7-max

_{Reviewed at 5c76c37f4692454258e971cb95aeb643937b09e6 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

Confidence: 5/5 — clean, minimal fix with a clear reproduction, verified before/after, and tests that match the real error shapes.

This is exactly the kind of PR that's easy to review and easy to maintain. The problem is real (linked issue with Node 22 repro), the fix is the minimum needed (unwrap one cause level, detect the killed/SIGTERM shape, reclassify ETIMEDOUT), and the before/after confirms it works end-to-end. In six months this code will read as obvious — "of course we check the cause" — which is the mark of a good bugfix.

No reservations. Ships it. ✅

中文说明

置信度：5/5 — 干净、最小化的修复，有明确复现、经验证的 before/after、以及与真实错误形态匹配的测试。

这正是容易审查、容易维护的 PR。问题真实（关联 issue 附 Node 22 复现），修复是最小必要改动（解包一层 cause、检测 killed/SIGTERM 形态、重分类 ETIMEDOUT），before/after 确认端到端有效。六个月后这段代码读起来会理所当然——"当然要检查 cause"——这就是好的 bugfix 的标志。

无保留意见。可以合入。✅

— Qwen Code · qwen3.7-max

_{Reviewed at 5c76c37f4692454258e971cb95aeb643937b09e6 · re-run with @qwen-code /triage}

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[yiliang114]

Corrected tmux E2E Test Report

Important

This replaces the earlier report's evidence boundary. The earlier Before/After used explicit qwen update, not the #7409 startup warning, and the raw Node execFile screenshot did not prove a complete Qwen timeout path. The corrected tests below use the real interactive startup TUI for both scenarios.

Result

• PASS — nested fetch cause: real startup changes from registry error to registry unreachable.
• NO DIFFERENTIAL — global-npm timeout: both revisions render registry did not respond within 5s.
• Follow-up fix(cli): narrow update-check error classification #7431 keeps the reproduced cause fix and removes the unsupported timeout classifications.

Environment: macOS 15.1.1, Node.js v22.22.0, tmux 3.5a, real interactive Qwen TUI at 200×50. Each run used an isolated empty QWEN_HOME plus --safe-mode; user settings and credentials were not read or changed.

Scenario 1: real startup background DNS failure

This was a normal interactive startup, not qwen update:

env QWEN_HOME=<isolated-dir> npm_config_registry=http://this-host-does-not-exist-zzz.invalid node dist/cli.js --safe-mode

Before — #7409 baseline 19ea6cba92	After — #7428 5c76c37f46

Before:

△ Update check skipped (registry error) — run /update to retry.

After:

△ Update check skipped (registry unreachable) — run /update to retry.

This is the reproduced user-visible improvement: Node 22's nested ENOTFOUND is classified as offline in the real #7409 startup path.

Scenario 2: real global-npm timeout control

Each revision was installed into an isolated temporary npm global layout and launched through the real production launcher. A local registry accepted the real package GET request and intentionally never responded.

Before — #7409 baseline 19ea6cba92	After — #7428 5c76c37f46

Observed on both revisions:

△ Update check skipped (registry did not respond within 5s) — run /update to retry.

The local server recorded one package request from each run. The requested baseline registry error → PR timeout differential was not reproducible. The outer fetchInfoWithTimeout and inner npm child timeout both use 5000 ms; the outer typed UpdateCheckTimeoutError surfaces before the killed child's rejection propagates. Baseline already classifies that typed error as timeout.

Verification

• Both tested revisions independently passed npm run build -- --cli-only and npm run bundle before their tmux runs.
• Follow-up fix(cli): narrow update-check error classification #7431: focused tests 40/40, changed-file ESLint, Prettier, CLI typecheck, and CLI build passed.
• Final Ponytail review: Lean already. Ship.

Screenshot provenance

macOS screen-capture permission was unavailable to the test process. Each PNG is visibly labeled and was rendered without content changes from its corresponding real tmux capture-pane output. The terminal text is included inline above for independent review.

[qwen-code-ci-bot]

Reviewed. Suggestions are inline.

— qwen3.7-max via Qwen Code /review

[qwen-code-ci-bot]

[Suggestion] Variable shadowing — the .some() callback parameter error shadows the outer function parameter error. Three distinct bindings named error coexist in this function (outer param at line 68, matchesCode callback here, and UpdateCheckTimeoutError .some() callback at line 80).

Concrete cost: a maintainer editing either callback body to reference the original error (e.g. error.name, error.stack, or a log statement) would silently reference the array element instead — producing a logic bug that passes existing tests because the array element is usually the same object.

Suggested change

	const matchesCode = (code: string) =>
	errors.some(
	(error) =>
	(error as NodeJS.ErrnoException).code === code ||
	error.message.includes(code),
	);
	const matchesCode = (code: string) =>
	errors.some(
	(e) =>
	(e as NodeJS.ErrnoException).code === code ||
	e.message.includes(code),
	);

Apply the same rename on line 80: errors.some((e) => e instanceof UpdateCheckTimeoutError).

— qwen3.7-max via Qwen Code /review

[wenshao]

Review — fix(cli): classify real update-check failure shapes

Nice, tightly-scoped fix. I reproduced both target error shapes on Node v22 to check the logic against reality, and the classifier now matches what Node actually produces:

• fetch DNS failure → top-level TypeError: fetch failed (no code) with cause.code === 'ENOTFOUND' on a single (non-aggregate) Error. The new one-level error.cause traversal picks this up. ✅
• execFile timeout → { code: null, killed: true, signal: 'SIGTERM' }. The killed/SIGTERM heuristic matches. ✅
• maxBuffer (the obvious false-positive worry) sets code: 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER' but leaves killed/signal unset, so the 'killed' in error guard correctly excludes it — it won't be misread as a timeout. 👍

Overview

classifyUpdateCheckError now (1) reads network codes from a direct error.cause, (2) treats a killed-by-SIGTERM npm child process as a timeout, and (3) reclassifies ETIMEDOUT from offline → timeout. Tests are updated to the observed fetch/execFile shapes.

Things worth calling out

• User-visible message change (intentional, but flag it): moving ETIMEDOUT out of NETWORK_ERROR_CODES flips its describeUpdateCheckFailure output from "registry unreachable" → "registry did not respond within 5s". That's arguably more accurate, but it is a behavior change for anyone whose socket connect currently fails with ETIMEDOUT — fine to keep, just noting it's not purely additive.

• AggregateError cause isn't traversed (already scoped out). Confirmed the primary Follow-up (#7409): update-check failure taxonomy misclassifies real offline/timeout errors #7423 DNS case is a single cause, so this fix covers it. The gap is the happy-eyeballs path (e.g. fetch → ECONNREFUSED across multiple addresses), where cause is an AggregateError carrying the code in cause.errors[] rather than on cause.code/cause.message; that still falls through to registry. You explicitly listed this as out of scope, so no objection — a cheap follow-up would be to spread cause.errors into the errors array when cause instanceof AggregateError.

Nits (non-blocking)

• Shadowing: the inner arrow params in matchesCode and the timeout errors.some(...) reuse the name error, shadowing the outer error. It's not a lint failure (no no-shadow rule is enabled), but renaming to e/candidate would read cleaner and avoids confusion with the outer binding used in the 'killed' in error check.

• Test coverage of the message-text path for timeouts: matchesCode also matches ETIMEDOUT embedded in an error message (the npm child-process stderr route), but only the code path is tested for timeout. offline has a dedicated "codes embedded in the message" test — adding the symmetric timeout-via-message case would fully cover the new branch.

Verdict

Correct, well-tested against real error shapes, and low-risk. Good to merge once you've eyeballed the ETIMEDOUT message change. The two nits are optional.

_{🤖 Reviewed with Claude Code (Opus 4.8, 1M context)}

[wenshao]

✅ Local build & real-run verification (on Linux)

I built this PR from source and verified the fix end-to-end. This notably covers Linux, which the PR marked ⚠️ (only macOS was manually verified in the description).

Environment: Linux 6.12 · Node v22.22.2 · bundled dist/cli.js from PR head 5c76c37

1 · Unit tests — all green

• packages/cli → vitest run src/ui/utils/updateCheck.test.ts → 41/41 passed (including the nested-cause fetch error, execFile timeout, and ETIMEDOUT cases).
• Consumer suites untouched by the change still pass: update.test.ts, update-command.test.ts, handleAutoUpdate.test.ts, startup-prefetch.test.ts → 89/89 passed.

2 · The mocked fixtures faithfully match REAL Node 22 error shapes

I generated genuine errors on this machine (not hand-written mocks) and ran them through the real classifier:

Real error (Node v22.22.2)	Observed shape	new classify	old (pre-PR)
fetch() DNS failure	TypeError: fetch failed, top code=undefined, cause.code=ENOTFOUND	offline ✅	registry ❌
execFile timeout	code=null, killed=true, signal=SIGTERM	timeout ✅	registry ❌
ETIMEDOUT	code=ETIMEDOUT	timeout ✅	offline ❌

The PR's new TypeError('fetch failed', { cause }) and Object.assign(…, { killed: true, signal: 'SIGTERM' }) fixtures reproduce the real shapes exactly — the fix reads error.cause and the child-process kill signal that the previous top-level-only classifier missed.

3 · Real end-to-end CLI run — unreachable registry (real DNS failure)

npm_config_registry=http://…zzz.invalid node dist/cli.js update

	Before (pre-PR classifier)	After (this PR)
EN	Failed to check for updates (registry error).	Failed to check for updates (registry unreachable).
ZH	检查更新失败（registry 错误）。	检查更新失败（registry 无法连接）。

Both exit with code 1. The "before" column was produced by reverting only the classifier and re-bundling, so the diff is attributable solely to this change.

Verdict: ✅ Behaves exactly as described in the PR, and now confirmed on Linux (which the PR left as ⚠️). The timeout/ETIMEDOUT paths are covered at the classifier level with genuine Node 22 error objects; the E2E run exercised the offline/DNS path. LGTM.

中文说明

✅ 本地构建 + 真实运行验证（Linux 平台）

我从源码构建了本 PR 并端到端验证了修复。这次特别覆盖了 Linux——PR 描述中该平台标记为 ⚠️（仅在 macOS 上做过手动验证）。

环境： Linux 6.12 · Node v22.22.2 · 从 PR head 5c76c37 bundle 出的 dist/cli.js

（截图见上方英文部分。）

1 · 单元测试——全部通过

• packages/cli → vitest run src/ui/utils/updateCheck.test.ts → 41/41 通过（包含嵌套 cause 的 fetch 错误、execFile 超时、ETIMEDOUT 三个新场景）。
• 未被本次改动触及的下游消费方测试同样通过：update.test.ts、update-command.test.ts、handleAutoUpdate.test.ts、startup-prefetch.test.ts → 89/89 通过。

2 · Mock 的错误形态与 Node 22 真实形态一致

我在本机生成了真实的错误对象（不是手写 mock），并喂给真实的分类器：

真实错误（Node v22.22.2）	观察到的形态	新 classify	旧（PR 前）
fetch() DNS 失败	TypeError: fetch failed，顶层 code=undefined，cause.code=ENOTFOUND	offline ✅	registry ❌
execFile 超时	code=null, killed=true, signal=SIGTERM	timeout ✅	registry ❌
ETIMEDOUT	code=ETIMEDOUT	timeout ✅	offline ❌

PR 中的 new TypeError('fetch failed', { cause }) 与 Object.assign(…, { killed: true, signal: 'SIGTERM' }) 精确复现了真实形态——修复读取了此前只看顶层 code 的分类器所遗漏的 error.cause 与子进程 kill 信号。

3 · 真实端到端 CLI 运行——registry 不可达（真实 DNS 失败）

npm_config_registry=http://…zzz.invalid node dist/cli.js update

	Before（PR 前分类器）	After（本 PR）
英文	Failed to check for updates (registry error).	Failed to check for updates (registry unreachable).
中文	检查更新失败（registry 错误）。	检查更新失败（registry 无法连接）。

两者退出码均为 1。"Before" 一列是仅回退分类器后重新 bundle 得到的，因此差异可完全归因于本次改动。

结论： ✅ 行为与 PR 描述完全一致，并已在 Linux 上确认（PR 中该平台为 ⚠️）。timeout/ETIMEDOUT 路径已在分类器层用真实 Node 22 错误对象覆盖，端到端运行覆盖了 offline/DNS 路径。LGTM。

---

_{🤖 Verified locally with Claude Code (Opus 4.8). This is a post-merge Linux verification record — the PR was already merged. Build/test artifacts are ephemeral.}

[qwen-code-ci-bot]

Released in v0.20.1.