fix(desktop): parse server ports strictly

[tt-a1i]

What this PR does

• Adds a shared strict parseServerPort helper for desktop server bootstrap ports.
• Uses it for CRAFT_RPC_PORT in standalone server bootstrap, standalone WebUI setup, and Electron embedded server env overrides.
• Uses it for CRAFT_HEALTH_PORT, parsing the health port before starting the standalone server.
• Adds parser unit coverage and standalone server startup regression coverage.

Why it's needed

parseInt accepts partial numbers. That means values like CRAFT_RPC_PORT=9100abc or CRAFT_HEALTH_PORT=3000abc could be treated as valid ports instead of failing fast. The server should only accept whole decimal TCP port numbers in the 0..65535 range.

Reviewer Test Plan

How to verify

• Review parseServerPort: it should accept whole decimal ports and reject trailing junk, fractions, exponent notation, negatives, and out-of-range values.
• Review standalone server startup: invalid CRAFT_HEALTH_PORT should be rejected before starting the WebSocket server.
• Review Electron embedded server startup: CRAFT_RPC_PORT env overrides should use the shared strict parser instead of parseInt.
• Run bun test packages/desktop/packages/server-core/src/bootstrap/__tests__/server-port.test.ts from the repo root.
• Run bun test packages/desktop/packages/server/src/__tests__/smoke.test.ts from the repo root.
• Run bun run typecheck from packages/desktop/packages/server-core.
• Run bun run typecheck from packages/desktop/packages/server.
• Run bun run build:main from packages/desktop/apps/electron.

Evidence (Before & After)

Before: parseInt('9100abc', 10) returned 9100, so malformed port env vars could silently bind a partially parsed port. After: CRAFT_RPC_PORT=9100abc and CRAFT_HEALTH_PORT=3000abc both exit non-zero with a clear invalid port error.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested locally; covered by CI
🐧 Linux	⚠️ not tested locally; covered by CI

Environment (optional)

Local desktop workspace after bun install --cwd packages/desktop --frozen-lockfile and root npm ci --ignore-scripts for tooling.

Risk & Scope

• Main risk or tradeoff: env values that used to be partially accepted are now rejected. Valid integer ports are unchanged.
• Not validated / out of scope: full desktop typecheck:all still fails in existing Electron files unrelated to this PR (src/main/auto-update.ts and src/main/handlers/__tests__/settings-default-thinking.test.ts).
• Breaking changes / migration notes: configs like 9100abc, 3000.5, or 1e3 must be changed to plain decimal port numbers.

Linked Issues

Fixes #5508

中文说明

What this PR does

• 新增 desktop server bootstrap 端口专用的严格 parseServerPort helper。
• 将 standalone server bootstrap、standalone WebUI 配置、Electron embedded server env override 里的 CRAFT_RPC_PORT 改成复用严格解析。
• 将 CRAFT_HEALTH_PORT 改成严格解析，并提前到 standalone server 启动前解析。
• 增加 parser 单测和 standalone server 启动级回归测试。

Why it's needed

parseInt 会接受部分数字。比如 CRAFT_RPC_PORT=9100abc 或 CRAFT_HEALTH_PORT=3000abc 可能被当成合法端口，而不是尽早失败。server 端口环境变量应该只接受 0..65535 范围内的完整十进制整数。

Reviewer Test Plan

How to verify

• 检查 parseServerPort：应接受完整十进制端口，拒绝尾随字符、小数、指数写法、负数和越界值。
• 检查 standalone server 启动：非法 CRAFT_HEALTH_PORT 应该在 WebSocket server 启动前被拒绝。
• 检查 Electron embedded server 启动：CRAFT_RPC_PORT env override 应该使用共享严格 parser，而不是 parseInt。
• 运行英文部分列出的测试和 typecheck/build 命令。

Evidence (Before & After)

修复前：parseInt('9100abc', 10) 返回 9100，格式错误的端口环境变量可能被静默截断并绑定到部分解析出的端口。修复后：CRAFT_RPC_PORT=9100abc 和 CRAFT_HEALTH_PORT=3000abc 都会非零退出，并打印清晰的 invalid port 错误。

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested locally; covered by CI
🐧 Linux	⚠️ not tested locally; covered by CI

Environment (optional)

本地 desktop workspace，已运行 bun install --cwd packages/desktop --frozen-lockfile；为了 tooling 也运行了 root npm ci --ignore-scripts。

Risk & Scope

• 主要风险或取舍：以前会被部分接受的 env 值现在会被拒绝。合法整数端口不受影响。
• 未验证 / 不在范围内：完整 desktop typecheck:all 仍在本 PR 无关的既有 Electron 文件失败（src/main/auto-update.ts 和 src/main/handlers/__tests__/settings-default-thinking.test.ts）。
• Breaking changes / migration notes：9100abc、3000.5、1e3 这类配置需要改成普通十进制端口数字。

Linked Issues

Fixes #5508

AI Assistance Disclosure

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

[wenshao]

📌 Maintainer note — consolidation & helper reuse (applies across your recent fix(...) series; anchoring here on #5509)

First, genuine thanks — the volume includes real, valuable fixes that are already merged (e.g. #5489 where FORCE_HYPERLINK=false was enabling hyperlinks, #5473 input-file truncation, #5504 ACP command wiring, #5452 http marketplace fetch). No concern about correctness.

Two process asks going forward, so review can keep up and we stop accumulating duplicate code:

1. Consolidate same-theme changes into one PR instead of one-PR-per-file. Each of these is really a single idea fanned across many files:

• case-insensitive URL scheme — fix(extensions): accept uppercase marketplace source schemes #5435, fix(extensions): handle uppercase npm registry schemes #5437, fix: accept uppercase endpoint URL schemes #5443, … (merged: fix(dingtalk): skip uppercase webhook reaction targets #5466, fix(desktop): accept uppercase icon URL schemes #5470)
• strict numeric/env parsing — fix(core): parse max output token env strictly #5491, fix(core): parse tool concurrency env strictly #5496, fix(desktop): parse server ports strictly #5509, …
• sibling-prefix path boundary — fix(core): enforce shell directory workspace boundary #5454, fix(core): allow dot-prefixed plans directories #5460, fix(desktop): enforce session plans path boundary #5507, …

One PR per theme (touching all relevant files) reviews as a unit and lands the same value.

2. Reuse the existing shared helpers instead of re-implementing them per file — this is already duplicating code in main:

• Port parsing: normalizePort (fix(core): validate lsp socket ports #5493) + parsePort (fix(desktop): parse NO_PROXY ports strictly #5498) + parseServerPort (this PR) = three near-identical "strict int in 1–65535" parsers. Please extract one shared util.
• Env integer parsing: parsePositiveIntegerEnvValue (fix(core): parse max output token env strictly #5491) and parsePositiveIntegerEnv (fix(core): parse tool concurrency env strictly #5496) are two copies of the same function.
• Path boundary: packages/core/src/utils/paths.ts already exports isSubpath / isPathWithinRoot; several PRs re-rolled a local isPathWithinDirectory. Please reuse the existing helper (or the desktop-shared equivalent for desktop PRs).
• http/https client by protocol: duplicated across fix(extensions): accept uppercase marketplace source schemes #5435 (marketplace.ts) and fix(extensions): handle uppercase npm registry schemes #5437 (npm.ts).

3. No need for a separate issue + PR per micro-fix — grouping related findings keeps the queue reviewable.

None of this knocks the work; it's about review throughput and a DRY codebase. Consolidated PRs will get fast-tracked. Thanks! 🙏

中文说明

📌 维护者说明 —— 合并同类项与复用 helper(适用于你最近这一系列 fix(...),以本 PR #5509 为锚点)

先真诚感谢:你提交的量里有不少是真有价值、已经合并的(比如 #5489 —— FORCE_HYPERLINK=false 反而开启了超链接、#5473 input-file 截断、#5504 ACP 命令注册、#5452 http marketplace 拉取)。代码正确性没有问题。

后续两点流程上的请求,目的是让 review 跟得上、并停止累积重复代码:

1. 同主题改动合并成一个 PR,而不是一个文件一个 PR。下面每一类其实都是同一个点子散在多个文件:

• URL scheme 大小写不敏感 —— fix(extensions): accept uppercase marketplace source schemes #5435、fix(extensions): handle uppercase npm registry schemes #5437、fix: accept uppercase endpoint URL schemes #5443、…(已合:fix(dingtalk): skip uppercase webhook reaction targets #5466、fix(desktop): accept uppercase icon URL schemes #5470)
• 数字/环境变量严格解析 —— fix(core): parse max output token env strictly #5491、fix(core): parse tool concurrency env strictly #5496、fix(desktop): parse server ports strictly #5509、…
• sibling 前缀路径边界 —— fix(core): enforce shell directory workspace boundary #5454、fix(core): allow dot-prefixed plans directories #5460、fix(desktop): enforce session plans path boundary #5507、…

每个主题一个 PR(覆盖相关文件)更容易作为整体审查,价值一样。

2. 复用已有的共享 helper,不要每个文件各写一份 —— 这已经在 main 里造成重复:

• 端口解析:normalizePort(fix(core): validate lsp socket ports #5493)+ parsePort(fix(desktop): parse NO_PROXY ports strictly #5498)+ 本 PR 的 parseServerPort,三份几乎一样的"1–65535 严格整数"解析。请抽一个共用 util。
• 环境变量整数解析:parsePositiveIntegerEnvValue(fix(core): parse max output token env strictly #5491)与 parsePositiveIntegerEnv(fix(core): parse tool concurrency env strictly #5496)是同一函数的两份。
• 路径边界:packages/core/src/utils/paths.ts 已导出 isSubpath / isPathWithinRoot;多个 PR 又各自重写了本地 isPathWithinDirectory。请复用现有 helper(desktop 的 PR 用 desktop 共享的对应实现)。
• 按协议选 http/https client:在 fix(extensions): accept uppercase marketplace source schemes #5435(marketplace.ts)与 fix(extensions): handle uppercase npm registry schemes #5437(npm.ts)里重复了。

3. 不必为每个微小修复单独开 issue + PR —— 把相关发现归并,队列才审得过来。

这些都不是否定你的工作,只是为了 review 吞吐合理、代码保持 DRY。整合后的 PR 会优先快速过。谢谢!🙏

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

On direction: strict port parsing is a legitimate robustness fix. parseInt('9100abc', 10) === 9100 silently accepting malformed ports is the kind of thing that causes confusing production failures. Aligned.

On approach: the implementation itself is clean and focused — good test coverage, correct validation, fail-fast behavior. However, this PR introduces the third near-identical port parser in the codebase:

• parsePort in packages/desktop/apps/electron/src/main/network-proxy-utils.ts — same regex /^\d+$/, same range check
• normalizePort in packages/core/src/lsp/LspConfigLoader.ts — same logic, private class method
• parseServerPort (this PR) in packages/desktop/packages/server-core/src/bootstrap/headless-start.ts

I see the existing functions are private to their modules with different error semantics (return undefined vs throw), so reuse isn't trivial. But adding a third copy moves in the wrong direction from the consolidation ask already flagged on this PR. A shared parsePortStrict util in packages/desktop (or even packages/core) that all three could delegate to would be ~10 lines and eliminate the drift.

Not blocking on this — the code is correct and the tests are solid. But worth thinking about whether a quick extraction PR (or follow-up commit here) makes sense before more callers appear.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

方向：严格端口解析是正确的健壮性修复。parseInt('9100abc', 10) === 9100 静默接受格式错误的端口会导致难以排查的生产故障。方向对齐。

方案：实现本身干净、聚焦——测试覆盖好、校验正确、快速失败。但本 PR 引入了代码库中第三个几乎一样的端口解析器：

• parsePort 在 network-proxy-utils.ts — 相同的正则 /^\d+$/，相同的范围检查
• normalizePort 在 LspConfigLoader.ts — 相同逻辑，私有类方法
• parseServerPort（本 PR）在 headless-start.ts

现有函数是各自模块私有的、错误语义不同（返回 undefined vs 抛异常），所以直接复用并不容易。但再加一份拷贝与已在本 PR 上标注的合并要求方向相反。在 packages/desktop（甚至 packages/core）抽一个共享的 parsePortStrict util，让三者都调用它，大约 10 行代码就能消除分化。

不作为阻塞项——代码正确、测试扎实。但值得考虑是否在这里加一个后续 commit（或单独的抽取 PR），在更多调用方出现前解决。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal: I'd add a strict port parser using /^\d+$/ + range check, place it in a shared util to avoid the third-copy problem, replace all parseInt calls on port env vars, add unit tests and startup regression tests.

The PR matches this approach in logic but places parseServerPort in headless-start.ts rather than a shared location. As flagged in Stage 1, this is the third near-identical port parser. Not a code correctness issue, but a maintenance debt item that will compound.

The implementation itself is clean:

• Regex validation catches partial parses (9100abc), floats (3000.5), exponents (1e3)
• Integer + range check (0–65535) catches negatives and out-of-range
• trim() handles whitespace in env values
• parseServerPortOrExit wrapper gives top-level server entry a clean fail-fast pattern
• Bootstrap correctly differentiates options.rpcPort (programmatic) from process.env.CRAFT_RPC_PORT (env var)
• Health port moved to top-level parse (was previously parsed near usage, now fails before any server startup)

One minor note: error message format differs between string path (JSON.stringify(raw) → "9100abc") and number path (String(raw) → 3000.5). Cosmetic, not blocking.

No correctness bugs, security holes, or regressions found.

Test Results

Unit tests (5/5 pass):

✓ parseServerPort > uses the default when the value is undefined
✓ parseServerPort > accepts whole decimal ports
✓ parseServerPort > accepts integer numeric ports from bootstrap options
✓ parseServerPort > rejects partially parsed port strings
✓ parseServerPort > rejects out-of-range or non-integer ports

Smoke tests (6/6 pass):

✓ headless server smoke test > accepts valid token handshake
✓ headless server smoke test > rejects invalid token
✓ headless server smoke test > rejects short token at startup
✓ headless server smoke test > rejects partially parsed RPC ports at startup
✓ headless server smoke test > rejects partially parsed health ports before starting
✓ headless server smoke test > shuts down cleanly on SIGTERM

Typecheck: server-core ✓, server ✓

Real-Scenario Testing (tmux)

Invalid CRAFT_RPC_PORT=9100abc

=== Test: CRAFT_RPC_PORT=9100abc ===
2026-06-20T21:56:07.967Z INFO  [bootstrap] Config artifacts initialized
2026-06-20T21:56:07.967Z INFO  [bootstrap] Global config found
Invalid CRAFT_RPC_PORT: expected an integer port between 0 and 65535, got "9100abc"
Exit: 1

Invalid CRAFT_HEALTH_PORT=3000abc

=== Test: CRAFT_HEALTH_PORT=3000abc ===
Invalid CRAFT_HEALTH_PORT: expected an integer port between 0 and 65535, got "3000abc"
Exit: 1

Valid ports (RPC=9100, HEALTH=0)

=== Test: Valid ports RPC=9100 HEALTH=0 ===
2026-06-20T21:56:13.912Z INFO  [bootstrap] Config artifacts initialized
2026-06-20T21:56:13.913Z INFO  [bootstrap] Global config found
2026-06-20T21:56:13.921Z INFO  Qwen Code server listening on ws://127.0.0.1:9100
CRAFT_SERVER_URL=ws://127.0.0.1:9100

All three scenarios behave exactly as expected — invalid ports fail fast with clear errors, valid ports start normally.

中文说明

代码审查

独立方案：用 /^\d+$/ + 范围检查实现严格端口解析，放在共享 util 避免第三份拷贝问题，替换所有 parseInt 调用，加单测和启动回归测试。

PR 的逻辑与方案一致，但 parseServerPort 放在 headless-start.ts 而非共享位置。如 Stage 1 所述，这是第三个近乎相同的端口解析器。不是正确性问题，但是会累积的维护债务。

实现本身干净：

• 正则校验捕获部分解析（9100abc）、小数（3000.5）、指数（1e3）
• 整数 + 范围检查（0–65535）捕获负数和越界
• trim() 处理环境变量中的空格
• parseServerPortOrExit 包装给顶层 server 入口提供干净的快速失败模式
• Bootstrap 正确区分 options.rpcPort（编程式）与 process.env.CRAFT_RPC_PORT（环境变量）
• Health 端口移到顶层解析（以前在使用处解析，现在在任何 server 启动前就失败）

一个小问题：错误信息格式在 string 路径（JSON.stringify → "9100abc"）和 number 路径（String → 3000.5）之间有差异。不影响功能。

未发现正确性 bug、安全漏洞或回归。

测试结果

单测 5/5 通过、冒烟测试 6/6 通过、类型检查通过。

真实场景测试

• CRAFT_RPC_PORT=9100abc → 退出 1，报错 Invalid CRAFT_RPC_PORT
• CRAFT_HEALTH_PORT=3000abc → 退出 1，报错 Invalid CRAFT_HEALTH_PORT
• 合法端口 → 正常启动 server listening on ws://127.0.0.1:9100

三种场景完全符合预期。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Verdict: Approve ✅

The fix is correct, well-tested, and does exactly what it says. Invalid port env vars now fail fast with clear errors instead of silently binding a partially-parsed port. Unit tests, smoke tests, typecheck, and real-scenario testing all pass.

The one reservation is the third-copy problem — parseServerPort joins parsePort and normalizePort as near-identical port parsers. The maintainer has already flagged this consolidation need in a prior comment. This isn't a blocker for the fix itself, but it's a real maintenance debt that should be addressed before more callers appear. A follow-up commit or PR to extract a shared parsePortStrict util would close the loop cleanly.

Recommending merge.

中文说明

结论：通过 ✅

修复正确、测试充分、行为完全符合描述。非法端口环境变量现在快速失败并给出清晰错误，而不是静默绑定部分解析出的端口。单测、冒烟测试、类型检查和真实场景测试全部通过。

唯一的顾虑是第三份拷贝问题——parseServerPort 加入了 parsePort 和 normalizePort 的行列，成为近乎相同的端口解析器。维护者已在之前的评论中标注了这一合并需求。这不是本修复的阻塞项，但是在更多调用方出现前应该解决的真实维护债务。后续 commit 或 PR 抽取共享 parsePortStrict util 可以干净地收尾。

建议合并。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. The fix is correct and well-tested. The port parser consolidation is a valid follow-up but doesn't block this merge. ✅

[wenshao]

@tt-a1i — quick note on the triage bot: @qwen-code /triage only re-triggers it when a maintainer posts the command. If you comment it on your own PR, the bot won't run, so there's no need to post it yourself. Once you've addressed the feedback (template / consolidation), just leave a comment saying it's ready and a maintainer will re-trigger triage for you. 🙏

中文说明

@tt-a1i —— 关于 triage bot 说明一下:@qwen-code /triage 只有维护者发出该命令时才会触发 bot 重新评审。你(贡献者)自己在 PR 里发是不会生效的,所以不用自己发。等你把反馈(模板 / 合并同类项)处理好后,留言说一声「ready」即可,维护者会替你重新触发 triage。🙏