fix(cli): parse FORCE_HYPERLINK strictly

[tt-a1i]

What this PR does

This PR parses FORCE_HYPERLINK strictly before force-enabling OSC 8 links. Only an empty string or a complete non-zero integer enables links; values such as false, off, and 1abc now fail closed instead of enabling links through NaN !== 0.

Why it's needed

Users commonly set boolean-looking environment values like FORCE_HYPERLINK=false. The old parser treated any non-empty non-zero parse result, including NaN, as enabled, so a value meant to disable links could unexpectedly force OSC 8 output.

Reviewer Test Plan

How to verify

Run the focused OSC 8 tests and confirm invalid or boolean-looking FORCE_HYPERLINK values no longer force hyperlink output, while the existing accepted values still work.

Evidence (Before & After)

Before: FORCE_HYPERLINK=false, off, or 1abc could force OSC 8 hyperlinks because invalid parses produced NaN, and NaN !== 0 evaluated true.

After: only an empty string or a complete non-zero integer force-enables hyperlinks; malformed values fail closed.

Tested on

OS	Status
🍏 macOS	✅ tested locally with focused unit tests
🪟 Windows	⚠️ not tested locally
🐧 Linux	⚠️ not tested locally

Environment (optional)

Local validation used the CLI package test runner for osc8.test.ts.

Risk & Scope

• Main risk or tradeoff: users relying on malformed values such as 1abc to force links will need to switch to a complete non-zero integer or empty value.
• Not validated / out of scope: no manual terminal rendering run; this is covered at the OSC 8 utility layer.
• Breaking changes / migration notes: no migration needed for documented values.

Linked Issues

Fixes #5487

Testing

• npm --workspace packages/cli test -- osc8.test.ts --coverage.enabled=false
• npx prettier --check packages/cli/src/ui/utils/osc8.ts packages/cli/src/ui/utils/osc8.test.ts docs/users/configuration/settings.md
• npx eslint packages/cli/src/ui/utils/osc8.ts packages/cli/src/ui/utils/osc8.test.ts
• npm --workspace packages/cli run typecheck
• npm run build -- --cli-only

Note: build completed with the existing Node v26 module.register() deprecation warning.

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

中文说明

这个 PR 做了什么

这个 PR 在强制启用 OSC 8 链接前严格解析 FORCE_HYPERLINK。现在只有空字符串或完整的非零整数会启用链接；false、off、1abc 这类值会 fail closed，不再因为 NaN !== 0 而启用链接。

为什么需要

用户经常会设置 FORCE_HYPERLINK=false 这类看起来像布尔值的环境变量。旧解析逻辑会把非空且解析结果不等于 0 的值当作启用，其中包括 NaN，导致本来想禁用链接的值反而强制输出 OSC 8。

Reviewer 测试计划

如何验证

运行聚焦的 OSC 8 测试，确认无效或布尔风格的 FORCE_HYPERLINK 不再强制启用链接，同时已有合法值仍然可用。

前后证据

之前：FORCE_HYPERLINK=false、off 或 1abc 可能强制启用 OSC 8，因为无效解析会得到 NaN，而 NaN !== 0 为 true。

之后：只有空字符串或完整非零整数会强制启用链接；畸形值会 fail closed。

测试平台

OS	状态
🍏 macOS	✅ 本地聚焦单测已验证
🪟 Windows	⚠️ 未本地验证
🐧 Linux	⚠️ 未本地验证

环境

本地验证使用 CLI package test runner，目标测试为 osc8.test.ts。

风险与范围

• 主要风险或取舍：如果有人依赖 1abc 这类畸形值强制启用链接，需要改用完整非零整数或空值。
• 未验证 / 不在范围内：没有做手动终端渲染验证；本 PR 在 OSC 8 工具层覆盖。
• 破坏性变更 / 迁移说明：文档化取值无需迁移。

关联 Issue

Fixes #5487

测试

• npm --workspace packages/cli test -- osc8.test.ts --coverage.enabled=false
• npx prettier --check packages/cli/src/ui/utils/osc8.ts packages/cli/src/ui/utils/osc8.test.ts docs/users/configuration/settings.md
• npx eslint packages/cli/src/ui/utils/osc8.ts packages/cli/src/ui/utils/osc8.test.ts
• npm --workspace packages/cli run typecheck
• npm run build -- --cli-only

说明：build 完成时仍有现存的 Node v26 module.register() deprecation warning。

AI Assistance Disclosure

I used Codex to review the changes, sanity-check the implementation against existing patterns, and help spot potential edge cases.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Hey @tt-a1i, thanks for the fix — the FORCE_HYPERLINK parsing bug is a real issue and the approach looks reasonable at first glance.

However, the PR body doesn't follow our pull request template. We need the standard sections to review efficiently:

• What this PR does / Why it's needed — motivation and change in prose
• Reviewer Test Plan — How to verify, Evidence (Before & After), Tested on (OS table), Environment
• Risk & Scope — tradeoffs, out-of-scope items, breaking changes
• Linked Issues — Fixes #5487 should go here (auto-close keyword)
• 中文说明 — bilingual summary

Could you reformat the body to match the template? The actual code change is small and focused — shouldn't take long to fill in. Happy to re-run triage once that's done.

中文说明

@tt-a1i 你好，感谢修复 FORCE_HYPERLINK 的解析 bug，方向没问题。

但 PR 正文没有按照我们的 PR 模板 填写。我们需要标准章节来高效审查：

• What this PR does / Why it's needed — 用文字描述改动和动机
• Reviewer Test Plan — 验证步骤、前后对比证据、测试过的系统、环境
• Risk & Scope — 取舍、不涉及的范围、破坏性变更
• Linked Issues — Fixes #5487 放在这里（自动关闭关键词）
• 中文说明 — 双语摘要

请按模板重新整理一下正文，代码本身改动很小，应该很快就能搞定。更新后我会重新跑 triage。

— Qwen Code · qwen3.7-max

[wenshao]

✅ Local verification — safe to merge

I verified this PR locally before merging. Tested PR head d7f9b617 on its base 56e76e7e, in an isolated git worktree with a clean npm ci (Node v22.22.2 / npm 10.9.7), with every step driven under tmux.

Results — all green

Check	Command	Result
osc8 unit tests	npm -w packages/cli test -- osc8.test.ts	✅ 81 passed (incl. the 2 new cases)
Format	prettier --check (osc8.ts, osc8.test.ts, settings.md)	✅ pass
Lint	eslint osc8.ts osc8.test.ts	✅ pass
Types	npm -w packages/cli run typecheck (tsc --noEmit)	✅ pass
Build	npm run build -- --cli-only	✅ pass

The fix is load-bearing (before / after)

Running the new regression test against the pre-fix osc8.ts:

× supportsHyperlinks > does not force hyperlinks for non-numeric FORCE_HYPERLINK values
  Tests  1 failed | 6 passed | 74 skipped (81)

That is the bug: the old parseInt(force, 10) !== 0 turns false / off into NaN !== 0 → true (and 1abc → 1 → true), force-enabling OSC 8. With shouldForceHyperlinks() in place it fails closed and the test passes. ✔️

Behavior is correct beyond the PR's own cases (extended check)

The PR's tests are real and non-mocked (they drive process.env + isTTY), so I extended them with 18 additional non-mocked cases through the public supportsHyperlinks(). All pass post-fix. A control case confirms a capable iTerm auto-enables when FORCE_HYPERLINK is unset — so the disable cases below genuinely come from the strict parse overriding auto-detection, not from an incapable terminal:

• enable (empty or complete non-zero integer): '', '1', '+1', '-1', '007', ' 1 ' (surrounding whitespace trimmed), '99999999999999999999'
• disable / fail-closed (asserted on a capable iTerm TTY): '0', '+0', '-0', 'false', 'off', 'true', '1abc', '1.5', ' ' (whitespace-only), '0x10'

Replaying that suite on the pre-fix code, exactly 6 regress — false, off, true, 1abc, 1.5, whitespace-only — which is precisely the bug surface. The fix corrects all six; the values that already parsed to 0 (0, +0, -0, 0x10) were unaffected either way.

Two design notes (non-blocking — both already handled correctly)

1. Fail-closed semantics: an invalid value doesn't merely "fail to force" — on an otherwise-capable terminal it actively disables (the FORCE_HYPERLINK branch returns before auto-detection). That's the right call for terminal escape output, and the docs update states it explicitly: "0 or a non-numeric value such as false / off to force-disable." Code, tests, and docs are aligned. 👍
2. Strict integer: non-integer numerics like 1.5 now disable (old parseInt → 1 → enabled). Consistent with the "complete integer" intent in bug(cli): FORCE_HYPERLINK=false enables OSC 8 links #5487.

Verdict

Correct, minimal, well-scoped, and fully covered by tests + docs. The bug is real and the fix closes it cleanly. LGTM 👍

🇨🇳 中文版（点击展开）

✅ 本地验证结论 —— 可以合并

合并前我在本地做了验证。在一个独立的 git worktree 中、基于 PR 头 d7f9b617（其基线为 56e76e7e）执行，使用全新的 npm ci（Node v22.22.2 / npm 10.9.7），并在 tmux 中运行每一步。

验证结果 —— 全部通过

检查项	命令	结果
osc8 单元测试	npm -w packages/cli test -- osc8.test.ts	✅ 81 个通过（含 2 个新增用例）
格式	prettier --check（osc8.ts、osc8.test.ts、settings.md）	✅ 通过
Lint	eslint osc8.ts osc8.test.ts	✅ 通过
类型	npm -w packages/cli run typecheck（tsc --noEmit）	✅ 通过
构建	npm run build -- --cli-only	✅ 通过

该修复确实是“关键且必要的”（前后对比）

把新增的回归测试跑在修复前的 osc8.ts 上：

× supportsHyperlinks > does not force hyperlinks for non-numeric FORCE_HYPERLINK values
  Tests  1 failed | 6 passed | 74 skipped (81)

这正是该 bug：旧的 parseInt(force, 10) !== 0 会把 false / off 变成 NaN !== 0 → true（1abc → 1 → true），从而强制启用 OSC 8。加入 shouldForceHyperlinks() 后会“失败即关闭”，测试通过。✔️

行为在 PR 自身用例之外也正确（扩展验证）

PR 的测试是真实、非 mock 的（直接设置 process.env 与 isTTY），所以我通过公共函数 supportsHyperlinks() 增补了18 个额外的非 mock 用例，修复后全部通过。其中一个对照用例确认：当未设置 FORCE_HYPERLINK 时，有能力的 iTerm 会自动启用——因此下面的“禁用”用例确实是严格解析覆盖了自动检测的结果，而非终端本身不支持：

• 启用（空串或完整的非零整数）：''、'1'、'+1'、'-1'、'007'、' 1 '（首尾空白被 trim）、'99999999999999999999'
• 禁用 / 失败即关闭（在有能力的 iTerm TTY 上断言）：'0'、'+0'、'-0'、'false'、'off'、'true'、'1abc'、'1.5'、' '（纯空白）、'0x10'

把这套用例回放到修复前的代码上，恰好有 6 个回归——false、off、true、1abc、1.5、纯空白——正是该 bug 的暴露面。修复将这 6 个全部纠正；而本就解析为 0 的值（0、+0、-0、0x10）在前后两版中都不受影响。

两点设计说明（不阻塞合并，且都已正确处理）

1. 失败即关闭语义：无效值并不是“没能强制启用”，而是在原本有能力的终端上主动禁用（FORCE_HYPERLINK 分支在自动检测之前就 return 了）。对终端转义输出而言这是正确取舍，且文档更新已明确写出：“0 或诸如 false / off 的非数值用于强制禁用。” 代码、测试、文档三者一致。👍
2. 严格整数：诸如 1.5 这类非整数数值现在会被禁用（旧的 parseInt → 1 → 启用）。与 bug(cli): FORCE_HYPERLINK=false enables OSC 8 links #5487 中“完整整数”的意图一致。

结论

正确、精简、范围清晰，且有测试与文档完整覆盖。bug 真实存在，此修复将其干净闭合。LGTM 👍

[wenshao]

[Suggestion] The comment 4 lines above (lines 179–181) claims this code "Mirrors the FORCE_HYPERLINK contract from supports-hyperlinks" but the new shouldForceHyperlinks function intentionally diverges by rejecting non-numeric values like false, off, 1abc — which is the whole point of this PR. A future maintainer who trusts that comment might "restore" the old parseInt behavior and silently re-introduce the bug.

Suggested change

	return shouldForceHyperlinks(force);
	// Explicit force overrides every heuristic below — but not the opt-outs
	// above nor the non-TTY guard. Empty string enables. Any string that is
	// entirely an optionally-signed integer with value ≠ 0 enables.
	// Non-numeric strings (e.g. 'false', 'off') disable.

— DeepSeek/deepseek-v4-pro via Qwen Code /review

[tt-a1i]

updated the PR description to match the template. thanks.

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR, @tt-a1i!

Template looks good ✓ — all required headings present, bilingual description, linked issue, test plan.

On direction: this is a textbook bug fix — FORCE_HYPERLINK=false force-enabling OSC 8 because parseInt('false', 10) returns NaN and NaN !== 0 is true. The bug is real, the user impact is clear (people commonly set boolean-looking env values), and it directly fixes #5487.

On approach: the scope is tight — one extracted helper (shouldForceHyperlinks), two test cases, and a docs update. Three files, +57/-28, zero drive-by changes. The regex-based strict integer validation is the right call for an environment variable that's documented as accepting "any non-zero integer." The settings.md table has some column-width realignment mixed in with the actual FORCE_HYPERLINK description update, but that's cosmetic noise, not scope creep.

Moving on to code review. 🔍

中文说明

感谢 PR，@tt-a1i！

模板完整 ✓ — 所有必需章节齐全，双语描述，关联了 issue，测试计划完整。

方向：这是一个典型的 bug 修复 — FORCE_HYPERLINK=false 因为 parseInt('false', 10) 返回 NaN 而 NaN !== 0 为 true，导致强制启用 OSC 8。bug 真实存在，用户影响明确（人们经常设置看起来像布尔值的环境变量），直接修复了 #5487。

方案：范围紧凑 — 提取了一个辅助函数（shouldForceHyperlinks），两个测试用例，以及文档更新。三个文件，+57/-28，没有顺手改动。基于正则的严格整数验证对于一个文档中写明接受"任意非零整数"的环境变量来说是正确的选择。settings.md 表格有一些列宽调整混在 FORCE_HYPERLINK 描述更新中，但那只是排版噪音，不算范围蔓延。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

The implementation is clean and correct. shouldForceHyperlinks() does exactly what it should: empty string → enable, regex-validated integer → enable if non-zero, everything else → disable. No over-abstraction, no unnecessary helpers, no code in the wrong place.

One minor note: the existing code comment at line ~178 still reads "Mirrors the FORCE_HYPERLINK contract from supports-hyperlinks: any non-zero numeric value (or empty string) enables" — after this PR, "numeric value" becomes "integer value" (e.g. 1.5 now disables). The comment's spirit is correct but the wording is slightly stale. Not a blocker, just something to tighten if you're touching the area.

Tests

Unit tests: 81 passed (including the 2 new cases) ✓
Format / Lint / Types: all clean ✓

Before/after behavior test — drove supportsHyperlinks() directly through 9 representative FORCE_HYPERLINK values on a simulated capable terminal (iTerm 3.5, TTY). The before run uses the old parseInt logic; the after run uses the PR's shouldForceHyperlinks().

Before (installed build — old parseInt logic)

BEFORE fix (main branch — parseInt logic)
==========================================
  ✓ FORCE_HYPERLINK=""         (empty string        ) => true  expected true
  ✓ FORCE_HYPERLINK="1"        (non-zero integer    ) => true  expected true
  ✓ FORCE_HYPERLINK="0"        (zero                ) => false expected false
  ✗ FORCE_HYPERLINK="false"    (boolean "false"     ) => true  expected false
  ✗ FORCE_HYPERLINK="off"      ("off"               ) => true  expected false
  ✗ FORCE_HYPERLINK="true"     ("true"              ) => true  expected false
  ✗ FORCE_HYPERLINK="1abc"     (partial numeric "1abc") => true  expected false
  ✗ FORCE_HYPERLINK="1.5"      (decimal "1.5"       ) => true  expected false
  ✗ FORCE_HYPERLINK="   "      (whitespace-only     ) => true  expected false

Results: 3 passed, 6 failed

After (this PR)

FORCE_HYPERLINK parsing test (PR #5489 fix)
============================================
  ✓ FORCE_HYPERLINK=""         (empty string        ) => true  expected true
  ✓ FORCE_HYPERLINK="1"        (non-zero integer    ) => true  expected true
  ✓ FORCE_HYPERLINK="0"        (zero                ) => false expected false
  ✓ FORCE_HYPERLINK="false"    (boolean "false"     ) => false expected false
  ✓ FORCE_HYPERLINK="off"      ("off"               ) => false expected false
  ✓ FORCE_HYPERLINK="true"     ("true"              ) => false expected false
  ✓ FORCE_HYPERLINK="1abc"     (partial numeric "1abc") => false expected false
  ✓ FORCE_HYPERLINK="1.5"      (decimal "1.5"       ) => false expected false
  ✓ FORCE_HYPERLINK="   "      (whitespace-only     ) => false expected false

Results: 9 passed, 0 failed

The fix corrects exactly the 6 cases that the old parseInt logic got wrong. The 3 values that already worked ('', '1', '0') are unaffected.

中文说明

代码审查

实现简洁且正确。shouldForceHyperlinks() 准确地做了该做的事：空字符串 → 启用，正则验证的整数 → 非零则启用，其他 → 禁用。没有过度抽象，没有不必要的辅助函数，代码位置正确。

一个小提示：约 178 行的现有代码注释仍然写着 "Mirrors the FORCE_HYPERLINK contract from supports-hyperlinks: any non-zero numeric value (or empty string) enables" — 这个 PR 之后，"numeric value" 变成了 "integer value"（例如 1.5 现在会禁用）。注释的精神是对的，但措辞略有过时。不是阻塞问题，只是既然改到了这个区域可以顺手修一下。

测试

单元测试： 81 个通过（含 2 个新增用例）✓
格式 / Lint / 类型： 全部通过 ✓

前后行为测试 — 在模拟的有能力终端（iTerm 3.5, TTY）上直接驱动 supportsHyperlinks()，覆盖 9 个有代表性的 FORCE_HYPERLINK 值。修复前使用旧的 parseInt 逻辑，修复后使用 PR 的 shouldForceHyperlinks()。

修复恰好纠正了旧 parseInt 逻辑出错的 6 个用例。原本就正确的 3 个值（''、'1'、'0'）不受影响。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

This is a clean, minimal bug fix that does exactly what it says. The bug is real (parseInt('false', 10) !== 0 → NaN !== 0 → true), the fix is correct (regex-validated strict integer before numeric comparison), and the before/after evidence confirms all 6 regression cases are resolved with zero side effects.

The implementation matches what I'd have written independently — extract the parsing into a named function, validate the input shape with a regex before doing numeric comparison, fail closed for anything that doesn't match. No over-engineering, no speculative features, no "let's also fix X while we're here."

The only thing worth a second thought is the slightly stale code comment about "any non-zero numeric value" — after this PR, it's "any non-zero integer." But that's a polish item, not a reason to hold up the merge.

LGTM. Approving. ✅

中文说明

这是一个干净、精简的 bug 修复，完全如描述所述。bug 是真实的（parseInt('false', 10) !== 0 → NaN !== 0 → true），修复是正确的（正则验证严格整数后再做数值比较），前后对比证据确认所有 6 个回归用例都已修复，零副作用。

实现方式与我会独立编写的方案一致 — 将解析逻辑提取为命名函数，在数值比较前用正则验证输入格式，对不匹配的输入 fail closed。没有过度工程，没有投机性功能，没有"顺便把 X 也修了"。

唯一值得再看一眼的是代码注释中"any non-zero numeric value"的措辞略显过时 — PR 之后应该是"any non-zero integer"。但这是打磨项，不是阻塞合并的理由。

LGTM。批准。✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅