fix(cli): validate ACP file read windows

[tt-a1i]

What this PR does

This PR validates ACP _qwen/file/read window parameters before calling the file system, and validates _qwen/file/read_bytes offset and maxBytes parameters before byte reads. It adds transport coverage for valid params, omitted defaults, and invalid window values.

Why it's needed

ACP file reads should reject malformed window values at the transport boundary instead of letting partial parses or invalid numeric values reach file-system logic. That keeps the method contract predictable for IDE and HTTP callers.

Reviewer Test Plan

How to verify

Run the focused ACP HTTP transport file-method tests. Confirm valid and omitted window params still work, while invalid startLine, limit, offset, or maxBytes values return validation errors before a file read is attempted.

Evidence (Before & After)

Before: malformed ACP file-read window params could pass through to the file-system call path.

After: invalid window params are rejected at dispatch/transport validation; valid and omitted defaults continue to work.

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 src/serve/acpHttp/transport.test.ts.

Risk & Scope

• Main risk or tradeoff: callers sending malformed numeric strings now receive validation errors instead of best-effort behavior.
• Not validated / out of scope: no real IDE client run; this is covered through ACP HTTP transport tests.
• Breaking changes / migration notes: valid requests keep the same behavior.

Linked Issues

Fixes #5481

Testing

• npm --workspace packages/cli run test -- src/serve/acpHttp/transport.test.ts -t "file methods"
• npm --workspace packages/cli run test -- src/serve/acpHttp/transport.test.ts
• npx prettier --check packages/cli/src/serve/acpHttp/dispatch.ts packages/cli/src/serve/acpHttp/transport.test.ts
• npx eslint packages/cli/src/serve/acpHttp/dispatch.ts packages/cli/src/serve/acpHttp/transport.test.ts
• npm --workspace packages/cli run typecheck
• npm run build -- --cli-only
• git diff --check

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 在调用文件系统前校验 ACP _qwen/file/read 的窗口参数，并在 byte read 前校验 _qwen/file/read_bytes 的 offset 和 maxBytes 参数。同时补充了 valid params、默认省略值和非法窗口参数的 transport 测试。

为什么需要

ACP 文件读取应该在 transport 边界拒绝畸形窗口值，而不是让部分解析或非法数字进入文件系统逻辑。这样 IDE 和 HTTP 调用方看到的方法合约更稳定。

Reviewer 测试计划

如何验证

运行聚焦的 ACP HTTP transport file-method 测试。确认合法和省略的窗口参数仍然可用，非法 startLine、limit、offset 或 maxBytes 会在读取文件前返回校验错误。

前后证据

之前：畸形 ACP file-read 窗口参数可能进入文件系统调用路径。

之后：非法窗口参数会在 dispatch/transport 校验处被拒绝；合法值和省略默认值行为不变。

测试平台

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

环境

本地验证使用 CLI package test runner，目标测试为 src/serve/acpHttp/transport.test.ts。

风险与范围

• 主要风险或取舍：发送畸形数字字符串的调用方现在会收到校验错误，不再走 best-effort 行为。
• 未验证 / 不在范围内：没有跑真实 IDE client；本 PR 通过 ACP HTTP transport 测试覆盖。
• 破坏性变更 / 迁移说明：合法请求行为不变。

关联 Issue

Fixes #5481

测试

• npm --workspace packages/cli run test -- src/serve/acpHttp/transport.test.ts -t "file methods"
• npm --workspace packages/cli run test -- src/serve/acpHttp/transport.test.ts
• npx prettier --check packages/cli/src/serve/acpHttp/dispatch.ts packages/cli/src/serve/acpHttp/transport.test.ts
• npx eslint packages/cli/src/serve/acpHttp/dispatch.ts packages/cli/src/serve/acpHttp/transport.test.ts
• npm --workspace packages/cli run typecheck
• npm run build -- --cli-only
• git diff --check

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]

Hi @tt-a1i — thanks for picking up #5481! The change itself looks focused and well-scoped from the diff, but the PR body doesn't follow the pull request template, which helps reviewers (and CI triage) assess PRs consistently.

Missing or renamed sections:

• ## What this PR does — currently ## Summary. The template asks for prose, not a bullet list of file-level changes.
• ## Why it's needed — missing entirely. The linked issue #5481 has the motivation, but the PR body should carry it too.
• ## Reviewer Test Plan — currently ## Test Plan. The template expects three subsections:
  • ### How to verify — steps a reviewer can follow to confirm the fix
  • ### Evidence (Before & After) — expected vs observed behavior (N/A is fine for non-TUI changes like this one)
  • ### Tested on — the OS table (🍏/🪟/🐧)
• ## Risk & Scope — missing. Even a one-liner ("low risk — validation only, no behavior change for valid inputs") helps.
• ## Linked Issues — Fixes #5481 is in the summary body but should be in its own section for auto-close.
• <details>中文说明</details> — missing.

Could you restructure the PR body to match the template? The content you have is good — it just needs to be in the right sections. Happy to help if anything is unclear.

中文说明

@tt-a1i 你好——感谢认领 #5481！从 diff 来看改动聚焦且范围合理，但 PR 正文没有按照 PR 模板 填写，这会影响审查效率和 CI 分流。

缺少或重命名的章节：

• ## What this PR does — 当前写的是 ## Summary，模板要求用散文描述而非文件级变更列表。
• ## Why it's needed — 完全缺失。关联 issue #5481 有动机说明，但 PR 正文也应包含。
• ## Reviewer Test Plan — 当前写的是 ## Test Plan，模板要求三个子章节：
  • ### How to verify — 审查者可以复现的步骤
  • ### Evidence (Before & After) — 预期 vs 实际行为（非 TUI 变更可写 N/A）
  • ### Tested on — 操作系统测试表格（🍏/🪟/🐧）
• ## Risk & Scope — 缺失。即使一句话（"低风险——仅增加校验，合法输入行为不变"）也有帮助。
• ## Linked Issues — Fixes #5481 写在 summary 里，应单独成节以触发自动关闭。
• <details>中文说明</details> — 缺失。

请按模板重新组织 PR 正文，内容本身没问题，只是需要放到正确的章节里。

— Qwen Code · qwen3.7-max

[tt-a1i]

updated the PR description to match the template. thanks.

[wenshao]

@qwen-code /triage

[wenshao]

[Suggestion] MAX_FILE_LINE_LIMIT = 2000 is a duplicate of the exported constant at packages/cli/src/serve/routes/workspaceFileRead.ts:37. If the REST route's value changes (e.g., raised to 3000), the ACP boundary silently retains 2000 — divergent enforcement on the same logical constraint.

Suggested change

	const MAX_FILE_LINE_LIMIT = 2000;
	import { MAX_FILE_LINE_LIMIT } from '../routes/workspaceFileRead.js';

(Remove the local const MAX_FILE_LINE_LIMIT = 2000; declaration.)

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

[wenshao]

[Suggestion] The new parameter validation rejects invalid params with INVALID_PARAMS errors, but produces zero server-side log output. If a client sends malformed params (e.g., maxBytes: 0), the file read fails silently from an operations perspective — there is no way to distinguish "request never arrived" from "request arrived but had invalid params" in server logs.

Consider adding this.logger?.warn(...) before each conn.sendConn(error(...)) call in the new validation blocks, logging the rejected parameter name, the invalid value, and the client ID.

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

[qwen-code-ci-bot]

Re-run after rebase onto main (conflict resolved).

Template ✓ — all sections present and properly structured.

Direction: defensive parameter validation at the ACP transport boundary, fixing #5481. Malformed maxBytes/line/limit/offset values were silently passing through to the FS layer instead of being rejected with INVALID_PARAMS. Clear API hygiene fix, aligned with the project's input-validation patterns. No direction concerns.

Approach: 2 files, +326/-25. One helper function, two validation insertion points, thorough tests. Scope is tight — no drive-by changes, no scope creep.

Moving on to code review. 🔍

中文说明

Rebase 到 main 后重新运行（冲突已解决）。

模板 ✓ —— 所有章节齐全且结构正确。

方向：在 ACP transport 边界做防御性参数校验，修复 #5481。畸形的 maxBytes/line/limit/offset 值之前会静默传递到 FS 层，而不是被 INVALID_PARAMS 拒绝。明确的 API 规范修复，与项目的输入校验模式一致，无方向性问题。

方案：2 个文件，+326/-25。一个辅助函数，两处校验插入点，测试全面。范围紧凑 —— 无夹带改动，无范围蔓延。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Re-run after rebase onto main. The diff is unchanged from the prior review.

parseOptionalSafeIntegerInRange is a clean, focused helper — three-state return (undefined for omitted → preserves downstream defaults, null for invalid → triggers INVALID_PARAMS, integer otherwise) maps well to the JSON-RPC contract. The existing parseOptionalPositiveInteger has different semantics (fallback-based, always positive), so a separate helper is justified.

Validation logic is correct across all edge cases: NaN, Infinity, fractional values, zero, negatives, string coercion, null, overflow beyond MAX_SAFE_INTEGER, and range boundaries — all properly rejected before fs.resolve() is called.

The error handling pattern (null-check + early return with error()) repeats per-param but matches the existing dispatch style — extracting it further would be over-engineering at this scope.

Test quality is solid: it.each with 15 cases for _qwen/file/read and 10 for _qwen/file/read_bytes, plus positive tests for forwarded params and omitted defaults. Tests assert both the error response AND that readText/readBytesWindow was never called — good defensive assertion.

makeFileFsFactory and resolvedPath test helpers are minimal and well-placed.

No issues found. Code is straightforward, tests are comprehensive.

Testing

This PR adds transport-layer JSON-RPC parameter validation — no TUI or user-visible behavior change. Validation is only reachable through ACP HTTP requests, not the CLI prompt, so tmux testing is not applicable here. The unit test suite is the correct verification path.

Focused test run (-t "file methods")

 ✓ src/serve/acpHttp/transport.test.ts (102 tests | 89 skipped) 541ms

 Test Files  1 passed (1)
      Tests  13 passed | 89 skipped (102)
   Duration  11.11s

Full transport test suite

 ✓ src/serve/acpHttp/transport.test.ts (102 tests) 5833ms

 Test Files  1 passed (1)
      Tests  102 passed (102)
   Duration  16.54s

Lint & typecheck

ESLint: clean (0 problems)
Typecheck: clean (tsc --noEmit, exit 0)

All 102 tests pass — no regressions in the transport suite.

中文说明

代码审查

Rebase 到 main 后重新运行。Diff 与上次审查一致。

parseOptionalSafeIntegerInRange 是一个干净的、聚焦的辅助函数 —— 三态返回（undefined 表示省略 → 保留下游默认值，null 表示非法 → 触发 INVALID_PARAMS，否则返回整数）很好地映射到 JSON-RPC 合约。已有的 parseOptionalPositiveInteger 语义不同（基于 fallback，始终为正），因此新增独立辅助函数是合理的。

校验逻辑对所有边界情况都正确：NaN、Infinity、小数、零值、负数、字符串强转、null、超出 MAX_SAFE_INTEGER 的溢出、范围边界 —— 均在 fs.resolve() 调用前被正确拒绝。

错误处理模式（null 检查 + error() 提前返回）对每个参数重复，但与现有 dispatch 风格一致 —— 在此范围下进一步提取会是过度工程。

测试质量扎实：_qwen/file/read 15 个用例、_qwen/file/read_bytes 10 个用例，加上验证合法参数转发和默认值保留的正向测试。测试同时断言了错误响应和底层函数未被调用 —— 良好的防御性断言。

未发现问题。代码简洁，测试全面。

测试

本 PR 添加的是 transport 层 JSON-RPC 参数校验 —— 无 TUI 或用户可见行为变更。该校验只能通过 ACP HTTP 请求触达，不通过 CLI 提示符，因此 tmux 测试在此不适用。单元测试套件是正确的验证路径。

全部 102 个测试通过 —— transport 测试套件无回归。ESLint 和 Typecheck 均无问题。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Re-run after rebase onto main. Diff unchanged, all checks still clean.

Stepping back: this PR does exactly what #5481 asks for, nothing more. The implementation is the most straightforward path — a single helper function, inline validation at the dispatch boundary, and thorough tests. My independent proposal before reading the diff would have been nearly identical: add range/type checks before the fs.readText() and fs.readBytesWindow() calls, return INVALID_PARAMS for bad values.

The code is boring in the best way — no abstractions, no cleverness, just careful parameter checking that matches the existing code style. The 102-test suite passing with no regressions confirms nothing was broken. Lint and typecheck are clean.

This is a clean fix from a first-time contributor. Approving. ✅

中文说明

Rebase 到 main 后重新运行。Diff 不变，所有检查仍然通过。

回顾：本 PR 完全按照 #5481 的要求实现，没有多余内容。实现路径最直接 —— 一个辅助函数、在 dispatch 边界内联校验、测试全面。我在阅读 diff 前的独立方案几乎一致：在 fs.readText() 和 fs.readBytesWindow() 调用前添加范围/类型检查，非法值返回 INVALID_PARAMS。

代码朴素到最好 —— 无抽象、无花哨，只是仔细的参数校验，与现有代码风格一致。102 个测试全部通过，无回归。Lint 和 Typecheck 均无问题。

这是一位首次贡献者的干净修复。批准 ✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Local real-build verification (maintainer) — tmux A/B against the real qwen serve ACP HTTP endpoint

Verdict: PASS — recommend merge. I built both sides from source, ran the real qwen serve /acp socket in tmux, and drove _qwen/file/read / _qwen/file/read_bytes end-to-end (initialize → SSE → method calls) against real files. The fix rejects every out-of-range / non-integer window param with a clean -32602 before touching the filesystem; the pre-fix build silently mis-reads or bypasses the line cap.

Method

• FIXED dist/cli.js @ e2604e88 (PR head) vs PRE-FIX @ 56e76e7e (origin/main) — both npm run build (exit 0).
• qwen serve --hostname 127.0.0.1 (loopback, auth-free) in an isolated tmux session, bound to a temp workspace with real files.
• A standalone Node client over the real socket: POST /acp initialize → GET /acp SSE → POST /acp method calls, response frames read off the SSE stream by id. No repo imports — this is the actual wire path an ACP client takes.

A/B at the real /acp socket (selected)

_qwen/file/read	PRE-FIX 56e76e7e	FIXED e2604e88
maxBytes:10,line:1,limit:1 (valid)	✅ "hello worl"	✅ "hello worl"
maxBytes:0	⚠️ OK, content "" (silent)	✅ -32602 `maxBytes` must be a positive integer in [1, 262144]
maxBytes:1.5	⚠️ OK, content "h" (silent truncation)	✅ -32602
limit:-5	⚠️ OK, content "" (silent)	✅ -32602 `limit` must be a positive integer in [1, 2000]
limit:5000 on a 3000-line file	⚠️ OK, returned 3001 lines (2000-line cap bypassed)	✅ -32602
line:0 / line:1.5	-32602 (already validated by readText)	-32602

_qwen/file/read_bytes	PRE-FIX	FIXED
offset:0,maxBytes:5 (valid)	✅ bytes hello	✅ bytes hello
offset:-1 / 1.5, maxBytes:0 / 262145	-32602 (already validated by readBytesWindow)	-32602

Findings

• ⚠️ The fix's real protective value is concentrated in _qwen/file/read's maxBytes + limit. Pre-fix silently returned empty / truncated content for 0 / negative / fractional values (no error at all — confirmed silent in the server log), and bypassed the 2000-line limit cap (limit:5000 → all 3001 lines returned = response-size amplification). The fix turns all of these into a clean -32602 with a precise message.
• 🔍 _qwen/file/read_bytes (offset, maxBytes) and _qwen/file/read's line were already validated downstream (readText/readBytesWindow reject with -32602). For these params the PR is defense-in-depth — it rejects earlier (before fs.resolve) and unifies the message wording. Harmless and tidier, but not a behavior change.
• 🔍 maxBytes is not a size-DoS vector for read: readText independently caps by file size ("file of N bytes exceeds read cap of 262144 bytes") — verified on a 300 KB file, where both builds reject. So the maxBytes validation is a correctness fix (reject 0 / negative / fractional), not a size cap. The genuine amplification fix is the limit cap above.
• 🔍 String params (maxBytes:"1"): pre-fix silently coerced to the default (full read); fixed rejects. Minor tightening, consistent with the rest.

Supplementary (deterministic)

• Unit (fixed): transport.test.ts "file methods" → 34 passed.
• Mutation: ran the PR's transport.test.ts against the pre-fix dispatch.ts → 25 of 27 invalid-param assertions FAIL → the new tests genuinely guard the validation (non-vacuous).

Tested on macOS (Node v22.22.2); CI covers Linux/Windows.

🇨🇳 中文版（点击展开）

✅ 维护者本地真实构建验证 —— 用 tmux 对真实 qwen serve 的 ACP HTTP 端点做 A/B

结论:PASS —— 建议合并。 我从源码构建了两侧,在 tmux 里跑起真实的 qwen serve /acp 套接字,并端到端地(initialize → SSE → 方法调用)对真实文件调用 _qwen/file/read / _qwen/file/read_bytes。修复后:所有越界 / 非整数的 window 参数都在碰文件系统之前就被干净地以 -32602 拒绝;而修复前的构建要么静默读错、要么绕过行数上限。

方法

• FIXED dist/cli.js @ e2604e88(PR head)对比 PRE-FIX @ 56e76e7e(origin/main)—— 两者都 npm run build(exit 0)。
• 在隔离的 tmux 会话里跑 qwen serve --hostname 127.0.0.1(loopback,免鉴权),绑定到一个含真实文件的临时工作区。
• 一个独立的 Node 客户端走真实套接字:POST /acp initialize → GET /acp SSE → POST /acp 方法调用,按 id 从 SSE 流上读回响应帧。不 import 仓库代码 —— 这正是一个 ACP 客户端实际走的链路。

真实 /acp 套接字上的 A/B(节选)

_qwen/file/read	PRE-FIX 56e76e7e	FIXED e2604e88
maxBytes:10,line:1,limit:1(合法)	✅ "hello worl"	✅ "hello worl"
maxBytes:0	⚠️ OK,内容为 ""(静默)	✅ -32602
maxBytes:1.5	⚠️ OK,内容为 "h"(静默截断)	✅ -32602
limit:-5	⚠️ OK,内容为 ""(静默)	✅ -32602
在 3000 行文件上 limit:5000	⚠️ OK,返回了 3001 行(2000 行上限被绕过)	✅ -32602
line:0 / line:1.5	-32602(readText 已校验)	-32602

_qwen/file/read_bytes	PRE-FIX	FIXED
offset:0,maxBytes:5(合法)	✅ 字节 hello	✅ 字节 hello
offset:-1 / 1.5、maxBytes:0 / 262145	-32602(readBytesWindow 已校验)	-32602

发现

• ⚠️ 修复的真正保护价值集中在 _qwen/file/read 的 maxBytes + limit。 修复前对 0/负数/小数会静默返回空 / 截断内容(完全没有报错 —— 我在服务端日志里确认是静默的),并且绕过了 2000 行的 limit 上限(limit:5000 → 返回全部 3001 行 = 响应体放大)。修复后这些都变成带精确提示的干净 -32602。
• 🔍 _qwen/file/read_bytes(offset、maxBytes)以及 _qwen/file/read 的 line 本来就在下游被校验(readText/readBytesWindow 已用 -32602 拒绝)。对这些参数,本 PR 属于纵深防御 —— 把拒绝提前到 fs.resolve 之前,并统一了提示措辞。无害、更整洁,但不是行为改变。
• 🔍 对 read 而言 maxBytes 不是 size-DoS 向量:readText 会按文件大小独立设限("file of N bytes exceeds read cap of 262144 bytes")—— 在 300 KB 文件上验证过,两侧都拒绝。所以 maxBytes 校验是正确性修复(拒绝 0/负数/小数),不是大小上限。真正的放大修复是上面的 limit。
• 🔍 字符串参数(maxBytes:"1"):修复前静默退化为默认值(整文件读取);修复后拒绝。轻微收紧,与其余一致。

补充(确定性)

• 单测(fixed): transport.test.ts "file methods" → 34 通过。
• 变异测试: 用 PR 的 transport.test.ts 跑修复前的 dispatch.ts → 27 个非法参数断言里有 25 个 FAIL → 新测试确实守护了该校验(非空泛)。

测试环境:macOS(Node v22.22.2);Linux/Windows 由 CI 覆盖。

_{Local real-build verification by the maintainer · FIXED e2604e88 vs base 56e76e7e · macOS / Node 22.22.2.}

[wenshao]

📌 Same maintainer note — full version on #5509.

Part of the strict-validation series. Heads-up: much of this path is already enforced downstream — invalid ACP read params already surface as INVALID_PARAMS via FsError in the FS layer — so please double-check what new coverage this actually adds before consolidating it into the single strict-parse/validate PR. Grouping + DRY, not a correctness objection.

中文说明

📌 同一条维护者说明 —— 完整版见 #5509。

属于严格校验系列。提醒:这条路径大部分在下游已经有校验了 —— 非法的 ACP read 参数已经会经 FS 层的 FsError 变成 INVALID_PARAMS —— 所以并入那个统一的严格解析/校验 PR 之前,请再确认本 PR 实际新增了哪些覆盖。这条是关于归并 + DRY,不是正确性反对。

[wenshao]

Heads-up: this PR now conflicts with main after recent merges (mergeable: CONFLICTING). It's already approved — could you rebase onto the latest main and resolve the conflicts so it gets back to a mergeable state? Thanks! 🙏

中文说明

提示:近期一些 PR 合并后,本 PR 与 main 产生了冲突(mergeable: CONFLICTING)。它已经 approved,能否 rebase 到最新的 main 并解决冲突,让它回到可合并状态?谢谢!🙏

[tt-a1i]

rebased onto latest main and pushed. conflict is gone on my side; local checks passed: transport.test.ts (131), cli lint/typecheck/build, git diff --check.

@qwen-code /triage

[wenshao]

@qwen-code /triage

[wenshao]

@tt-a1i — also a note on triage: @qwen-code /triage only re-triggers the bot when a maintainer posts the command; posting it yourself on your own PR won't run it. So after you rebase this one onto main and resolve the conflicts, no need to post /triage yourself — just leave a comment that it's ready and a maintainer will re-trigger triage for you. 🙏

中文说明

@tt-a1i —— 顺带说明 triage:@qwen-code /triage 只有维护者发出该命令才会触发 bot;你自己在 PR 里发是不会生效的。所以本 PR rebase 到 main、解决冲突之后,不用自己发 /triage——留言说一声「ready」即可,维护者会替你重新触发 triage。🙏

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Maintainer verification — real end-to-end test (Linux)

Verified this PR locally with the real qwen serve daemon (not just unit tests), driven over the wire under tmux. Verdict: LGTM — safe to merge. All four verification layers pass, and this also closes the Linux gap left open in the PR's "Tested on" table.

Environment: isolated git worktree + clean npm ci · PR head 69502a3d · Node v22.22.2 · Linux 6.12.63 (Debian 13).

Summary

#	Check	Result
A	PR's own file-method tests (transport.test.ts -t "file")	42 passed / 0 failed
B	Full transport.test.ts (regression guard)	131 passed / 0 failed
C	A/B toggle — disable the fix, re-run	25 fail (exactly the reject cases) → restore → 42 pass
D	Real daemon end-to-end over the wire (tmux)	26/26 passed
E	prettier --check + eslint on both changed files	clean

---

A/B proof — the new tests actually guard the fix (and reproduce #5481)

I neutralized parseOptionalSafeIntegerInRange to mimic the pre-fix behavior (pass any number through, treat non-numbers as "omitted", never reject) and re-ran:

fix DISABLED → 25 failed | 89 skipped
  ✓ _qwen/file/read forwards valid window parameters
  ✓ _qwen/file/read preserves defaults when window parameters are omitted
  × _qwen/file/read rejects invalid window params ({"maxBytes":"1"})   ← issue #5481 case
  × ... 14 more read cases (maxBytes/line/limit × type/range/fraction/null)
  × _qwen/file/read_bytes rejects invalid window params ({"offset":-1})
  × ... 9 more read_bytes cases (offset/maxBytes)
fix RESTORED → 42 passed | 0 failed

The 25 failures are exactly the "rejects invalid" cases; the "valid" and "omitted-default" cases keep passing. This confirms the tests are meaningful (not vacuously green) and demonstrates the precise bug from #5481 — e.g. maxBytes: "1" was silently accepted before, rejected after.

Real daemon end-to-end (the "real test")

Booted the actual binary inside a tmux session against a real file on disk (no mocks — real TCP, real HTTP, real SSE, real filesystem):

qwen serve --no-web --hostname 127.0.0.1 --port 41784 --workspace /tmp/pr5482-ws
  → listening, /acp + WebSocket transport enabled, bearer auth disabled (loopback)
workspace file: sample.txt = "line1-AAAA\nline2-BBBB\nline3-CCCC\nline4-DDDD\nline5-EEEE\n" (55 bytes)

A real HTTP/JSON-RPC client initialized a connection, opened the connection-scoped SSE stream, and fired 26 requests. Replies (over SSE):

id  method      expect  got         verdict detail
----------------------------------------------------------------------------------
100 read        accept  ok          PASS    content="line1-AAAA\nline2-BBBB\nline3-…"
101 read        accept  ok          PASS    content="line2-BBBB" (line=2 limit=1)
102 read        accept  ok          PASS    content="line1" truncated (maxBytes=5)
110 read        reject  err(-32602) PASS    `maxBytes` must be a positive integer in [1, 262144]   ← "1" (string)
111 read        reject  err(-32602) PASS    `maxBytes` ... in [1, 262144]   ← 0
112 read        reject  err(-32602) PASS    `maxBytes` ... in [1, 262144]   ← 262145 (MAX+1)
113 read        reject  err(-32602) PASS    `maxBytes` ... in [1, 262144]   ← 1.5
114 read        reject  err(-32602) PASS    `maxBytes` ... in [1, 262144]   ← null
120-123 read    reject  err(-32602) PASS    `line` must be a positive integer   ← 0 / "2" / 1.5 / >MAX_SAFE_INT
130-133 read    reject  err(-32602) PASS    `limit` ... in [1, 2000]   ← 0 / 2001 / "1" / null
200 read_bytes  accept  ok          PASS    sizeBytes=55 returnedBytes=55 offset=0 sha256:5330c3a2…
201 read_bytes  accept  ok          PASS    sizeBytes=55 returnedBytes=5 offset=6 truncated=true
210-213 bytes   reject  err(-32602) PASS    `offset` must be a non-negative safe integer   ← -1 / "1" / 1.5 / null
220-223 bytes   reject  err(-32602) PASS    `maxBytes` ... in [1, 262144]   ← "1" / 0 / 262145 / 1.5
----------------------------------------------------------------------------------
SUMMARY: 26/26 passed, 0 failed

Server-side, the daemon pane logged the matching POST /acp … status=202 acks for connection e3bbf189. Two things this proves beyond the unit tests:

• Valid windows are actually honored against the real FS — line=2,limit=1 returns line2-BBBB; maxBytes=5 truncates to line1; byte offset=6,maxBytes=5 returns a real 5-byte slice with a real sha256.
• Malformed params are rejected at the dispatch boundary with -32602 and the exact messages in dispatch.ts — the filesystem read (resolve/readText/readBytesWindow) is never reached.

Minor (non-blocking) observation

In both handlers fsFactory.forRequest(...) is created before the window-param validation, so it is allocated even on the reject path. It performs no filesystem I/O (the actual access is resolve/readText/readBytesWindow, which are correctly gated), so this is a negligible allocation, not a correctness issue. Optional: move it after validation for symmetry. Not a merge blocker.

Conclusion

The change brings ACP _qwen/file/read / _qwen/file/read_bytes to the documented contract (maxBytes ∈ [1,262144], limit ∈ [1,2000], line positive safe int, offset non-negative safe int), rejecting malformed params before touching the FS while leaving valid/omitted behavior unchanged. Verified at the unit, A/B, and live-daemon levels on Linux. 👍

🇨🇳 中文版（点击展开）

✅ 维护者验证 —— 真实端到端测试（Linux）

我在本地用真实的 qwen serve 守护进程（不只是单元测试）验证了此 PR，并在 tmux 下通过真实网络请求驱动。结论：LGTM，可以合并。 四个验证层全部通过，同时补上了 PR "Tested on" 表中尚未验证的 Linux 一栏。

环境： 独立 git worktree + 全新 npm ci · PR head 69502a3d · Node v22.22.2 · Linux 6.12.63（Debian 13）。

汇总

#	检查项	结果
A	PR 自带 file-method 测试（transport.test.ts -t "file"）	42 通过 / 0 失败
B	完整 transport.test.ts（回归保护）	131 通过 / 0 失败
C	A/B 开关 —— 关掉修复后重跑	25 失败（正好是 reject 用例）→ 还原 → 42 通过
D	真实守护进程端到端（tmux 真实网络请求）	26/26 通过
E	对两个改动文件跑 prettier --check + eslint	干净

A/B 证明 —— 新测试确实在守护这个修复（并复现 #5481）

我把 parseOptionalSafeIntegerInRange 改成修复前的行为（数字直接放行、非数字当作"未传"、永不拒绝）再重跑：

关闭修复 → 25 失败 | 89 跳过
  ✓ valid window parameters 仍通过
  ✓ omitted defaults 仍通过
  × rejects invalid ({"maxBytes":"1"})   ← 正是 issue #5481 的例子
  × ……其余 14 个 read 用例 + 10 个 read_bytes 用例
还原修复 → 42 通过 | 0 失败

这 25 个失败正好是"拒绝非法参数"用例，而"合法"与"省略默认值"用例始终通过。这说明测试是有意义的（不是空过），并复现了 #5481 的确切 bug —— 例如 maxBytes: "1" 修复前被静默接受、修复后被拒绝。

真实守护进程端到端（"真实测试"本体）

在 tmux 会话里启动真实二进制，针对磁盘上的真实文件运行（无 mock —— 真实 TCP / HTTP / SSE / 文件系统）：

qwen serve --no-web --hostname 127.0.0.1 --port 41784 --workspace /tmp/pr5482-ws
  → 监听中，/acp + WebSocket 已启用，loopback 免鉴权
sample.txt = "line1-AAAA\n…\nline5-EEEE\n"（55 字节）

真实 HTTP/JSON-RPC 客户端 initialize 建连、打开连接级 SSE 流，发出 26 个请求，结果（经 SSE 返回）26/26 全部通过：

• 合法窗口参数对真实文件真正生效：line=2,limit=1 → line2-BBBB；maxBytes=5 → 截断为 line1；字节 offset=6,maxBytes=5 → 返回真实 5 字节片段且带真实 sha256。
• 所有畸形参数在 dispatch 边界以 -32602 拒绝，错误信息与 dispatch.ts 完全一致 —— 文件读取（resolve/readText/readBytesWindow）从未被触达。

守护进程侧日志记录了连接 e3bbf189 对应的 POST /acp … status=202。这比单元测试多证明了两点：合法窗口确实作用于真实文件系统；非法参数在到达文件层之前即被拦截。

次要（不阻塞）观察

两个 handler 都在窗口参数校验之前就创建了 fsFactory.forRequest(...)，因此即便走拒绝分支也会分配一次。它不做任何文件 I/O（真正的访问是 resolve/readText/readBytesWindow，这些已被正确拦截），所以只是一次可忽略的分配，并非正确性问题。可选：把它移到校验之后以保持对称。不阻塞合并。

结论

此改动使 ACP _qwen/file/read / _qwen/file/read_bytes 符合文档约定（maxBytes ∈ [1,262144]、limit ∈ [1,2000]、line 为正安全整数、offset 为非负安全整数），在触碰文件系统前拒绝畸形参数，同时合法/省略行为保持不变。已在单元、A/B、真实守护进程三个层面于 Linux 上验证。👍

_{Verification was scripted against a real loopback daemon; no model/auth involved (file-read methods don't touch the LLM). Test artifacts were run in a throwaway worktree + temp workspace.}