fix(serve): allow bounded reads of large text files - #7947
Conversation
Serve large-text range read verificationGoalVerify that Scenarios
Results
Commandscd packages/core
npx vitest run src/utils/read-text-range.test.ts src/services/fileSystemService.test.ts
cd ../cli
npx vitest run src/serve/fs/workspace-file-system.test.ts \
src/serve/bridge-file-system-adapter.test.ts \
src/serve/routes/workspace-file-read.test.ts
cd ../..
npm run typecheck
npm run buildThe manual model-flow plan remains in |
65d9924 to
748c888
Compare
github-actions
Bot
added
the
review/self-reported
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
…s set
Follow-up to the bounded large-text read path. Three changes:
Gate on any explicit window argument, not on `limit`. Gating on `limit`
had the cost model backwards in both directions: `{ line: 900_000_000,
limit: 20 }` was admitted despite walking the whole file, while
`{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A
read with no window argument at all still fails, since a caller that
believes it holds the whole file may write it back truncated.
Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read
returns; nothing capped what it cost. Line offsets are resolved by
scanning from byte 0, so a query param could turn into an
uninterruptible multi-second scan of an arbitrarily large file — and on
Windows hold a read handle for that span, blocking renames and deletes.
Past the budget the read is refused with `file_too_large` pointing at
readBytes, which reaches any offset in O(1).
Tolerate appends on streamed windows. Requiring whole-file size/mtime
stability after reading a prefix rejected reads whose returned bytes
were still valid, and the case it rejected — tailing a live log — is the
one this path exists for. Streamed windows now assert inode identity
plus "did not shrink"; truncation and replacement are still rejected.
Also: non-UTF-8 large text now returns `binary_file` rather than
`file_too_large`, so a client retrying on 413 with a smaller window
can't loop forever; and `readFileWithLineAndLimit` throws instead of
silently ignoring a caller-supplied `fileHandle` on the by-path
fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Self-review notes on e784e6d — three changes to the read policy, plus the gaps I'm knowingly leaving.
Why the gate moved off limit
The original gate asked "did the caller pass a finite limit?" But what a read costs isn't the limit — line offsets are resolved by scanning from byte 0, so limit says nothing about how far we walk. Gating on it got both directions wrong:
| Request | Actual cost | Old behavior |
|---|---|---|
line: 900000000, limit: 20 on 10 GB |
full scan, ~33 s | admitted |
maxBytes: 4096 |
first 4 KiB | file_too_large |
line: 2 (no limit) |
output capped at 256 KiB anyway | file_too_large |
Now any explicit window argument (line / limit / maxBytes) admits the read, and MAX_TEXT_SCAN_BYTES = 8 MiB bounds the cost independently of which knob was set. A read with no window argument still fails — editText/writeTextAtomic already can't fire without a hash (which partial windows omit), but writeTextOverwrite takes no hash, so refusing the windowless read is what keeps a caller from ever holding a silently-truncated "whole file".
The budget is checked when the next chunk arrives rather than after consuming the current one: reaching that point proves there was more file left, which is what separates "ran out mid-file" from "the file ended on the budget", without trusting a possibly-stale stats.size.
Why streamed windows now tolerate appends
Requiring whole-file size/mtimeMs stability after reading a prefix rejects reads whose returned bytes are still valid — and the case it rejected is the one this PR exists for. Appending to a log does not change lines 1-20, but under an equality check every read of a live log was a coin flip. The old test rejects in-place changes while a large range is being read was asserting exactly that failure with appendFile.
Streamed windows now assert inode identity plus "did not shrink"; truncation and replacement are still rejected, and sizeBytes reports the size at open so it describes the snapshot the window was cut from. The residual gap is a writer that truncates and regrows past the original size inside one read window on the same inode — narrower than what the equality check caught, and the price of supporting append-only files at all.
Error kind
Large non-UTF-8 text now returns binary_file (422) instead of file_too_large (413). Shrinking the window can never make a GBK file decodable, so a client with the obvious 413 retry policy looped forever; readBytes is the correct remedy and binary_file already points there.
Verification
| Suite | Result |
|---|---|
core src/utils + src/services |
6077 passed |
cli src/serve/fs/ + bridge adapter |
222 passed |
tsc (core full, cli touched files), eslint, prettier |
clean |
Not run: routes/workspace-file-read.test.ts doesn't load in my checkout — @qwen-code/channel-github fails to build, which also breaks it on the unmodified base. Neither that file nor the ACP dispatch tests assert 413 on large reads, so the gate change doesn't invalidate their expectations, but I could not execute them.
Known gaps, not addressed here
- Line-offset paging is still O(n²) across pages. Reading lines 1-20, then 21-40, … of a large log rescans from byte 0 each time. The real fix is a byte cursor — accept
offsetin bytes and returnnextOffsetpast the last whole line, the text-aware sibling of whatreadBytesalready does. That's a wire-schema addition, so it belongs in its own PR; the scan budget is what makes the current shape safe until then. - Encoding detection is forked, not threaded.
detectFileHandleEncodingduplicatesdetectFileEncoding; the right shape is one function takingFileHandle | string. Left alone to keep this diff to policy. - Windows is still untested. The scan budget caps the handle-hold window at ~25 ms of streaming, which is most of what I was worried about (Node opens without
FILE_SHARE_DELETE, so a held read handle blocks renames), but I have not run the suite there.
| // the same bound apply, and raises `TextScanBudgetExceededError` if the | ||
| // window really is out of reach. | ||
| if ( | ||
| request.forceStreaming !== true && |
There was a problem hiding this comment.
Note for reviewers: the fileHandle !== undefined && forceStreaming !== true combination is unreachable from every in-repo caller — readTextFileFromHandle always sets forceStreaming: true — so readFileHandleBuffer below has no coverage. AGENTS.md:243 calls out this shape (a switch no caller can set).
I left it because collapsing it means reshaping the API into an explicit readTextRangeFromHandle(fh, req) rather than two optional booleans on a shared path, which is a bigger change than this diff should carry. Flagging it so it isn't mistaken for exercised code.
There was a problem hiding this comment.
Noted — left as-is per the author's documented decision. Collapsing the unreachable fileHandle !== undefined && forceStreaming !== true combination requires reshaping the read API into an explicit handle-entry, which is out of scope for this diff (AGENTS.md Simplicity First). No change this round.
中文说明
已记录——按作者已记录的决定保持不变。折叠不可达的 fileHandle !== undefined && forceStreaming !== true 组合需要将读取 API 重塑为显式的 handle 入口，超出本 diff 的范围（AGENTS.md 简洁优先）。本轮不作更改。
| let position = 0; | ||
| while (true) { | ||
| signal?.throwIfAborted(); | ||
| const buffer = Buffer.allocUnsafe(highWaterMark); |
There was a problem hiding this comment.
512 KiB is above Buffer.poolSize, so each iteration is a real allocation — roughly 2000 per GB scanned. Each chunk is fully decoded by the consumer before the next read resolves, so hoisting one buffer out of the loop would be safe.
Less pressing now that MAX_TEXT_SCAN_BYTES caps a boundary scan at 8 MiB (16 allocations), but the read_file tool path has no budget and can still walk a whole file.
There was a problem hiding this comment.
Noted — already addressed in the final code: readFileHandleChunks allocates one 512 KiB Buffer.allocUnsafe before the loop and yields subarray views into it, so there is no per-iteration allocation (this note predates the final commit). The new yield-site comment now documents that reuse contract. No change needed.
中文说明
已记录——最终代码已解决：readFileHandleChunks 在循环前分配一个 512 KiB 的 Buffer.allocUnsafe，并 yield 指向它的 subarray 视图，因此不存在逐次分配（此说明早于最终提交）。新增的 yield 处注释现已记录该复用契约。无需更改。
| const meta: TextReadOutcome['meta'] = { | ||
| encoding: readMeta?.encoding, | ||
| bom: readMeta?.bom, | ||
| lineEnding: readMeta?.lineEnding ?? detectLineEnding(content), |
There was a problem hiding this comment.
Worth knowing that lineEnding means something slightly different on each path: the snapshot path derives it from the returned slice, while the streamed path latches 'crlf' as soon as any scanned chunk contains CRLF — including lines before the window, which are never returned. So a file with a CRLF header and an LF body can report crlf for an LF-only window.
Defensible either way since it describes the file rather than the excerpt, but both feed the same meta.lineEnding field.
There was a problem hiding this comment.
Noted — informational. The streamed path latches crlf from any scanned chunk (describing the file), while the snapshot path derives it from the returned slice; both are defensible and feed the same meta.lineEnding. Left as-is to keep this diff to policy. No change this round.
中文说明
已记录——信息性说明。流式路径会从任何被扫描的块中锁定 crlf（描述整个文件），而快照路径从返回的切片中推导；两者都站得住脚，并都写入同一个 meta.lineEnding。为将本 diff 限定在策略层面，保持不变。本轮不作更改。
| await fh.close(); | ||
| } | ||
|
|
||
| if (opened === undefined) { |
There was a problem hiding this comment.
This branch is unreachable — opened is assigned by await fh.stat() as the first statement in the try, and a rejection there propagates instead of falling through. Kept only because the | undefined in the declaration forces TS to see it as possibly unset after the block.
There was a problem hiding this comment.
Noted — left as-is per the author's note. The opened === undefined branch is unreachable (a stat() rejection propagates rather than falling through), but the | undefined in the declaration is kept so TypeScript narrows correctly after the try; removing it is a type-shape change beyond this diff's scope. No change this round.
中文说明
已记录——按作者的说明保持不变。opened === undefined 分支不可达（stat() 的 reject 会向上传播而非顺势 fall through），但保留声明中的 | undefined，以便 TypeScript 在 try 之后正确收窄类型；移除它属于超出本 diff 范围的类型形态变更。本轮不作更改。
|
Thanks for the PR! Template looks good ✓ Problem: observed bug with clear evidence. Issue #7946 documents that Direction: aligned. Agents using the Serve workspace filesystem currently fall back to shell commands for large text files, which defeats the purpose of the sandboxed filesystem boundary. Building on the existing Size: core paths touched ( Approach: the scope feels right. The PR adds a narrow streaming branch for large text when any explicit window argument is present ( Risk: no elevated risk signals (no high-risk path matches). Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：已观测到的 bug，有明确证据。Issue #7946 记录了 方向：对齐。使用 Serve 工作区文件系统的 agent 目前不得不回退到 shell 命令读取大文本文件，这违背了沙箱文件系统边界的初衷。基于现有 规模：触及核心路径。生产逻辑 589 行，测试 538 行，文档 70 行。 方案：范围合理。新增窄流式分支，复用现有 风险：无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
|
Code reviewIndependent proposal: I would have added a streaming branch in Comparison: the PR matches this closely and extends it in a reasonable way — any of No critical blockers found. Specific observations:
sequenceDiagram
participant C as Caller
participant W as WorkspaceFileSystem
participant P as Policy
participant L as StandardFileSystemService
participant F as FileHandle
C->>W: readText(path, opts)
W->>W: lstat - reject symlink and non-regular
alt file le 256 KiB
W->>W: full snapshot path (unchanged)
else file gt 256 KiB AND window arg present
W->>F: open and stat
W->>W: assertSameFile and assertDidNotShrink
W->>P: binary probe (4 KiB)
W->>L: readTextFileFromHandle
L->>F: stream lines (cap output 256 KiB, scan 8 MiB)
L-->>W: content and meta
W->>F: stat again, assertSameFile, assertDidNotShrink
W->>F: close
W->>W: lstat, assertSameFile, assertDidNotShrink
else windowless large read
W-->>C: file_too_large
end
W-->>C: content, meta (no hash for partial windows)
Files changed (14 of 14 shown)
Testing evidenceThis is an unattended CI run — no PR code was executed by the triage agent. Evidence below is from the PR's own CI checks and the automated serve A/B test. The serve A/B test (built base vs head The main unit test suite ( Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Not verified: Windows and Linux manual runs (author tested on macOS only). The 中文说明代码审查独立方案： 在 对比： PR 与独立方案高度一致，并合理扩展—— 未发现关键阻断问题。TOCTOU 保护全面，扫描预算检查最多超一个 512 KiB 块（已记录），测试覆盖全面（538 行新测试）。 测试证据无人值守 CI 运行——triage agent 未执行 PR 代码。证据来自 PR 自身的 CI 检查和自动化 serve A/B 测试。 serve A/B 测试确认现有端点无回归。主单元测试套件（Ubuntu）仍在运行中。macOS 和 Windows 测试已跳过。 — Qwen Code · qwen3.8-max-preview Reviewed at |
|
Confidence: 3/5 — clean review across every stage, but 589 production lines touching core paths ( This is a well-executed fix for a real usability gap. The problem is clearly observed (issue #7946, labeled My independent proposal matched the PR's approach closely. The one extension I'd highlight as a good design call: accepting any of What keeps this at 3/5 rather than higher: the two-tier gate requires maintainer sign-off for core changes at this scale, and the main CI suite ( @doudouOUC — nice work. The append-tolerant stability model and the residual-gap documentation in @yiliang114 @wenshao — this PR touches core filesystem service and utils at 589 production lines. The review found no blockers, but the two-tier gate needs your sign-off before merge. CI was still running at review time; the finalize workflow will update the CI table when it settles. 中文说明置信度：3/5 —— 各阶段审查均无问题，但 589 行生产代码触及核心路径（ 这是一个执行良好的修复，解决了真实的可用性问题。问题有明确观测（issue #7946，标记 独立方案与 PR 方案高度一致。值得指出的良好设计：接受 保持 3/5 而非更高的原因：两级门禁要求维护者对此规模的核心变更签字，且主 CI 套件（Ubuntu）在审查时仍在运行。两者均不反映代码质量问题。 — Qwen Code · qwen3.8-max-preview Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.7-max via Qwen Code /review
| sizeBytes: opened.size, | ||
| truncated: true, |
There was a problem hiding this comment.
[Suggestion] truncated is hardcoded to true for every large-file streaming read, even when the returned content was not actually truncated. — Failure scenario: a file just above MAX_READ_BYTES (262,145 bytes) with few lines and a limit that covers them all produces originalLineCountExact: true and truncatedByBytes: false, but the response still carries truncated: true. A caller using this flag to decide whether it holds the complete file will issue a wasteful follow-up read for content it already has. The small-file path (readTextSnapshotFromResolvedFile) sets truncated only when byte or line truncation actually occurred, so the two paths disagree on the flag's meaning.
| sizeBytes: opened.size, | |
| truncated: true, | |
| sizeBytes: opened.size, | |
| truncated: readMeta?.originalLineCountExact !== true || readMeta?.truncatedByBytes === true, |
中文说明
[Suggestion] 对于所有大文件流式读取，truncated 被硬编码为 true，即使返回的内容实际上并未被截断。—— 失败场景：一个刚好超过 MAX_READ_BYTES（262,145 字节）的文件，行数很少，且 limit 能覆盖所有行时，会产生 originalLineCountExact: true 和 truncatedByBytes: false，但响应仍然带有 truncated: true。使用此标志判断是否已获取完整文件的调用方会发起一次多余的后续读取。小文件路径（readTextSnapshotFromResolvedFile）仅在字节或行截断实际发生时才设置 truncated，因此两条路径对该标志的含义不一致。
— qwen3.7-max via Qwen Code /review
There was a problem hiding this comment.
[Suggestion] Declined — the hardcoded truncated: true is the documented contract for large partial windows, not an oversight.
Three concrete reasons:
- The protocol doc (
docs/developers/qwen-serve-protocol.md,GET /file) states large partial windows "settruncated: true". The streaming path only fires for files above the 256 KiB cap and never returns the whole file, so every window is partial by construction. - The suggested expression would flip the existing
beyondEofassertion inworkspace-file-system.test.ts: a past-EOF window hasoriginalLineCountExact: trueandtruncatedByBytes: false, so the expression yieldsfalsewhile the test deliberately assertstrue. - The stated failure scenario cannot occur:
maxBytesis validated to[1, MAX_READ_BYTES], so a fully-covered window of a file above the cap always exceeds the output cap and is byte-truncated (truncatedByBytes: true).
If a maintainer prefers computed truncation semantics here, it should land as a deliberate contract change that updates the protocol doc and the beyondEof expectation together.
中文说明
[Suggestion] 驳回——硬编码的 truncated: true 是大文件部分窗口已被文档约定的契约，并非疏漏。
三个具体原因：
- 协议文档（
docs/developers/qwen-serve-protocol.md的GET /file）说明大文件部分窗口会"设置truncated: true"。流式路径仅在文件超过 256 KiB 上限时触发，且从不返回整个文件，因此每个窗口按构造都是部分的。 - 建议的表达式会翻转
workspace-file-system.test.ts中现有的beyondEof断言：越过 EOF 的窗口具有originalLineCountExact: true与truncatedByBytes: false，因此该表达式得出false，而测试刻意断言为true。 - 所述失败场景不可能发生：
maxBytes被校验为[1, MAX_READ_BYTES]，因此超过上限的文件被完整覆盖的窗口总会超出输出上限并被按字节截断（truncatedByBytes: true）。
如果维护者希望此处采用计算出的截断语义，应作为一个有意的契约变更，同时更新协议文档与 beyondEof 预期。
|
Review note against HEAD The body says a read "with only a starting line, and with only a byte cap … should retain the existing Two independent verification results from a clean detached checkout of
No code-level findings beyond the four self-annotations already on the diff. Core suites (70 tests), full build and typecheck pass at HEAD. |
Treat caller-owned file handles as bounded streaming reads, cap them to the captured file size, and reuse the chunk buffer. Restore strict Serve snapshot stability and align returned-slice metadata with the full-snapshot path. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Reviewed and reconciled the follow-up commit The follow-up moved in the right direction by binding reads to one handle,
Final commit Two consecutive broad audits are clean. Focused verification is 238/238, with |
|
@qwen-code /verify |
|
Sandboxed verification: merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. 沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 Scripted assertions: 59 passed · 0 failed · 59 total 脚本断言：59 通过 · 0 失败 · 59 总计 Verification report (report.md)Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
…ger (QwenLM#7965) Post-merge measurement of QwenLM#7917, one day in: 9 eligible PRs, two considered-and-declined mentions (both correct calls), zero positive recommendations. The one clear behavioural candidate — QwenLM#7947, bounded reads of large text files — wrote "Not verified: Windows and Linux manual runs (author tested on macOS only)" in its own Stage 2 comment and never named a lane. That is the failure shape worth fixing: the judgement-based rule ("when neither static review nor 2b substantiates it") failed exactly where the comment had already written the gap down in so many words. The model judged that pending CI would cover it; a green suite proves the tests pass, not that the untested behaviour holds. So the trigger is now textual, not judgemental: before posting, grep your own draft. A sentence of the shape "not verified", "author tested on one platform only", or "author's claim, not independently re-run" IS the trigger — the 2b-bis line is that same sentence with the remedy attached, and omitting it means telling the maintainer what is missing while withholding the one command that would supply it. Pending CI does not lift the trigger. The two legitimate skip cases (nothing behavioural to settle; author lacks write) are unchanged, and the rule is ordered before them so they read as outs from the requirement, not the requirement as an out from them. The trigger phrases are verbatim from real comments: "not verified" and "author tested on macOS only" from QwenLM#7947, "author's claim, not independently re-run" from QwenLM#7951. Mutation-verified 3/3: dropping the trigger paragraph, moving it after the skip cases, and dropping the pending-CI sentence each turn the test red. n=1 is thin evidence for a behavioural rule change — but this rule is text-matching, not probability-weighing, so it cannot overfit to the sample that motivated it. Co-authored-by: wenshao <wenshao@example.com>
…wenLM#7947) Address review feedback on the large-text range read PR: - Pause before restoring mtime in the two ctime-dependent mutation tests so the change-time advances past the pre-read snapshot even on coarse-resolution filesystems, removing a latent flake in the same-size-overwrite precondition. The assertions are unchanged. - Document at the readFileHandleChunks yield site that the 512 KiB buffer is reused across iterations, so yielded views must be decoded or copied before advancing the generator.
|
🤖 Addressed the latest review feedback (round 1/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 1/100 轮）。改动内容与我反驳保留之处如下： Review feedback triageTriaged the seven inline findings newer than the last evaluation. Implemented the Implemented
Declined
Informational — author self-review notes (no change requested)
ConflictNo conflict with main ( Verification
No settings source changed, so 中文说明评审反馈分诊分诊了自上次评估以来新增的七条行内发现。实现了测试 flakiness 的 Critical 与文档类 Suggestion；驳回了一条与已被文档约定的线上契约相冲突的 Suggestion；确认了四条不要求更改的作者自审说明。 已实现
已驳回
信息性——作者自审说明（不要求更改）
冲突与 main 无冲突（ 验证
未更改任何 settings 源，因此无需 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
Local verification of
|
| Gate | Result |
|---|---|
core read-text-range + fileSystemService |
71/71 |
cli workspace-file-system + bridge adapter + HTTP route |
166/167 |
eslint --max-warnings 0, 6 changed production files |
clean |
core tsc --noEmit |
25 errors, all in 3 unrelated test files, identical count on base |
The one CLI failure is throws AggregateError when every workspace root glob fails, which fails identically on the base commit — it assumes an unprivileged filesystem and I run as root. The tsc errors come from an @types/node skew in the borrowed node_modules, not from this PR; none touch a changed file. Repo-wide typecheck and build are covered by CI, which is green on this head.
Verdict
Behaviour matches the description on every scenario I could construct, the central change is provably load-bearing, and the new tests fail for the right reasons when it is removed. No blocker from me. For the record before merge: the 422 binary_file flip and the sub-cap UTF-16 truncation are user-visible contract changes, and the same-size + mtime-restore rejection is best-effort at coarse-clock resolution rather than absolute.
中文说明
本地验证 dfd941e —— 两个真实守护进程、真实文件、无 mock
我在本地实际构建并运行了这个 PR，而不是只看测试套件。环境：两个真实的 qwen serve 守护进程，base b6b55c5 跑在 :4792，PR head dfd941e 跑在 :4791，都直接执行 TypeScript 源码，绑定同一个工作区和同一批 fixture（417 KB ASCII CSV、497 KB UTF-8 中文 CSV、14.3 MB 日志、300 KB 二进制、809 KB GBK、554 KB UTF-16LE、BOM+CRLF、超长中文/emoji 行，以及两个小于上限的对照文件）。下面所有数字都来自这两个进程。
1. 真实 agent 回合中的问题与修复
（见上方第一张截图）
脚本化模型对 417 KB CSV 发起 read_file(file_path, offset: 0, limit: 20)。在 base 上模型收到 file of 417145 bytes exceeds read cap of 262144 bytes —— issue #7946 被端到端复现，路径经过 ACP bridge 进入 WorkspaceFileSystem。在 PR head 上同一次调用返回 Showing lines 1-20 of 6002 total lines. 以及具体内容。
反向对照。 在 head 构建上只回退 if (pre.size > MAX_READ_BYTES && opts.limit !== undefined) 这一个条件，REST 路由和 agent 工具调用都精确回到 base 的行为；同一次回退还让 PR 新增的 12 个测试以行为断言失败（file_too_large 对 binary_file / symlink_escape / hash_mismatch，413 对 200），而不是导入或编译错误。说明这些测试确实钉住了本次改动。
2. GET /file A/B —— 23 个场景
（见上方第二张截图）
- 准入是收窄而不是放宽。 无窗口、仅 line、仅 maxBytes、line 加 maxBytes 在两侧都保持
413。 - 深偏移可用。 14.3 MB 文件的第 110 000 行（约 10.5 MB 处）约 37 ms 返回。
- 输出仍然有界。
maxBytes=1000精确返回 1000 字节；262144接受，262145与0两侧都是parse_error。 - 元数据与描述一致。
truncated: true、完整的源文件sizeBytes、无全文件hash；扫描提前停止时originalLineCount为null，扫描到 EOF 时为精确值6002。 - 编码路径正确。 大型 UTF-8 中文文件被放行；BOM 与 CRLF 上报正确；超长中文/emoji 行按码点边界截断，无替换字符。大型 GBK 与 UTF-16LE 仍为
413，并带"转成 UTF-8"的提示。 - 小于上限的对照文件在 base 与 head 上逐字节一致。
有两处是有意为之、但对现有客户端可见的变化，希望合入前明确确认：
- C1 —— 大于 256 KiB 的二进制文件带
limit读取，从413 file_too_large变为422 binary_file。分类更准确，但对该类输入而言是错误类型的变化。 - G3 —— 一个 235 KB 的 UTF-16LE 文件，此前解码后返回 333 293 字节 UTF-8，超过 262 144 字节的边界上限；现在被截断并标记
truncated: true。方向正确，但它作用在一个小于上限的文件上：忽略truncated的调用方会静默地只拿到原本完整文件的约 80%。建议在 release notes 里写一句。
3. 快照一致性
（见上方第三张截图）
-
通过真实守护进程驱动的并发修改： 在 14.3 MB 文件扫描过程中执行 append、truncate、符号链接替换 —— 36/36 全部拒绝（
409 hash_mismatch、400 symlink_escape），没有出现混合快照。 -
dfd941e修掉的 flaky 确实修好了。 在696a8d4上，两个 ctime 测试在本机连续 10 次整文件运行中分别失败 7/10 与 9/10；在dfd941e上是 0/10。起作用的正是那个+50 ms暂停。 -
生产侧的残余边界。 同大小覆盖并恢复 mtime 的情形依靠
ctimeMs检测，而它按内核粗粒度时钟推进 —— 本机实测量子为 4.0 ms，tmpfs 与 ext4 均为 195/200 次 ctime 相同。当覆盖写落在与open相同的量子内时，23/30 次读取返回了包含 open 之后写入字节的窗口；加 50 ms 延迟后 30/30 被拒绝。这不是回归 —— base 直接拒绝这类读取，而既有的全量快照路径只比较
size+mtimeMs，新路径严格更强。我唯一的请求是措辞：PR 描述里写的是这类覆盖"在返回内容前被拒绝"，建议改成"当变更落在更晚的时间戳量子时被拒绝"，避免下游把它当成硬保证。
4. 各项门禁
| 门禁 | 结果 |
|---|---|
core read-text-range + fileSystemService |
71/71 |
cli workspace-file-system + bridge adapter + HTTP route |
166/167 |
eslint --max-warnings 0，6 个改动的生产文件 |
通过 |
core tsc --noEmit |
25 个错误，全部位于 3 个无关测试文件，base 上数量完全相同 |
唯一失败的 CLI 用例是 throws AggregateError when every workspace root glob fails，它在 base 提交上同样失败 —— 该用例假设非特权文件系统，而我以 root 运行。tsc 错误来自借用 node_modules 造成的 @types/node 版本偏差，与本 PR 无关，且不涉及任何被改动的文件。全仓 typecheck 与 build 由 CI 覆盖，当前 head 上 CI 全绿。
结论
我能构造出的所有场景，行为都与 PR 描述一致；核心改动被证明是 load-bearing；移除它时新增测试以正确的原因失败。我这边没有阻断项。 合入前留档两点：422 binary_file 的变化与小于上限的 UTF-16 截断属于对用户可见的契约变更；同大小覆盖加恢复 mtime 的拒绝是粗粒度时钟精度下的尽力而为，而非绝对保证。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.8-max-preview via Qwen Code /review
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Review feedback addressed — PR #7947SummaryThe only feedback this round is @wenshao's issue-level local-verification report (two real daemons, real files, no mocks). It is positive — "No blocker from me" — with no inline comments, no reviews, and no failed checks. It carries three soft asks: acknowledge two user-visible contract changes (C1, G3) and soften the same-size-rewrite rejection wording so nobody treats it as a hard guarantee. One resulted in a small docs change; the other two are acknowledgments with no in-repo artifact to edit. Feedback points and decisionsWording: same-size + mtime-restore rejection is best-effort, not absolute — AddressedThe reviewer asked to soften "rejected before content is returned" to "rejected whenever the change lands in a later timestamp quantum." The PR-description text itself is maintained by the workflow (this bot has no GitHub write access), but the committed design doc this PR added ( C1 — >256 KiB binary read with a
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
No issues found. LGTM! ✅
中文说明
未发现问题。LGTM！✅
— qwen3.8-max-preview via Qwen Code /review
…vior reversals (QwenLM#7967) Maintainer review adjudicated the pending design-reversal Critical: the descriptor-threading refactor is sound and boundary-neutral, but this PR also carried two undisclosed reversals of QwenLM#7947 that must not ride in on a refactor. Revert both, keeping only the refactor and the disclosed non-UTF-8 mapping: - Restore didFileVersionChange (size + mtime + ctime equality before and after the streamed read) in place of assertDidNotShrink. The size-only guard let a same-size in-place overwrite return torn content and a concurrent append run past the snapshot it opened against; the equality check rejects both with hash_mismatch. Main's three snapshot tests are restored unchanged. - Restore QwenLM#7947's limit-only large-text admission and drop MAX_TEXT_SCAN_BYTES / maxScanBytes / TextScanBudgetExceededError. The broadened wantsWindow admission and scan budget are a real contract change that belongs in its own PR against QwenLM#7947, not here. Kept: the descriptor-threading refactor (one detectFileEncoding, two entry points, readTextFileFromHandle/readTextRangeFromHandle, no mode flags) and the disclosed mapping of large non-UTF-8 windows to binary_file rather than file_too_large, which the maintainer explicitly approved.
* fix(serve): allow bounded reads of large text files
* fix(serve): bound large-text reads by scan cost, not by which knob was set
Follow-up to the bounded large-text read path. Three changes:
Gate on any explicit window argument, not on `limit`. Gating on `limit`
had the cost model backwards in both directions: `{ line: 900_000_000,
limit: 20 }` was admitted despite walking the whole file, while
`{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A
read with no window argument at all still fails, since a caller that
believes it holds the whole file may write it back truncated.
Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read
returns; nothing capped what it cost. Line offsets are resolved by
scanning from byte 0, so a query param could turn into an
uninterruptible multi-second scan of an arbitrarily large file — and on
Windows hold a read handle for that span, blocking renames and deletes.
Past the budget the read is refused with `file_too_large` pointing at
readBytes, which reaches any offset in O(1).
Tolerate appends on streamed windows. Requiring whole-file size/mtime
stability after reading a prefix rejected reads whose returned bytes
were still valid, and the case it rejected — tailing a live log — is the
one this path exists for. Streamed windows now assert inode identity
plus "did not shrink"; truncation and replacement are still rejected.
Also: non-UTF-8 large text now returns `binary_file` rather than
`file_too_large`, so a client retrying on 413 with a smaller window
can't loop forever; and `readFileWithLineAndLimit` throws instead of
silently ignoring a caller-supplied `fileHandle` on the by-path
fallback.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(core): thread the descriptor instead of forking text-read helpers
PR QwenLM#7947 pinned large-text reads to one inode by threading a caller-owned
FileHandle into readTextRange as an optional field, plus a second field,
forceStreaming, to suppress the buffering fast path. Two optional fields
produced four combinations: one meaningful, one used by a single test, one
unreachable, and — in readFileWithLineAndLimit — one that silently fell
through to a by-path read, defeating the reason the caller opened a handle.
Unify the two encoding detectors. detectFileEncoding now takes a path or a
borrowed handle, so detectFileHandleEncoding is deleted along with the
message discrepancy between them: an encoding iconv-lite cannot load now
raises LargeNonUtf8TextError naming that encoding rather than deferring to
the decoder's generic invalid-utf8 variant. Both still refuse the file, and
the Serve boundary maps both to binary_file.
Split the reader into readTextRange (path) and readTextRangeFromHandle
(always streams, both byte bounds required). The unreachable combination and
its untested readFileHandleBuffer are gone, and with no fileHandle parameter
left for readFileWithLineAndLimit to ignore, the RangeError guarding that
fallthrough is deleted too — the trap can no longer be expressed.
CoreReadTextFileHandleRequest drops its required stats field. Nothing
downstream read it, and because the ACP request type it extends permits
extra properties, TypeScript accepted the dead argument silently.
readFileHandleChunks becomes chunksFromHandle(fh, from) — the one seam
byte-cursor text paging needs.
No observable change at the Serve boundary: its 222 tests pass unmodified.
Two fileSystemService tests were deleted rather than repaired; they asserted
the arguments readFileWithLineAndLimit received, which is nothing once the
handle path stops calling it. Their coverage lives in read-text-range.test.ts
against real files and in workspace-file-system.test.ts at the real boundary.
258 production lines in core, net -71 overall.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* refactor(core): make CoreReadTextFileHandleRequest standalone
Self-audit follow-up to f55c867. Two fields survived the reshape that the
handle path never reads:
- `stats` was documented as required ("must pass the Stats captured from that
handle") and nothing downstream read it. The handle path always streams, so
it never needs a size to choose a strategy, and the encoding probe does its
own fstat.
- `path` became dead once readTextRangeFromHandle replaced the path-plus-handle
call. Errors are labelled with the path by the Serve boundary that owns it.
Neither was caught by the compiler: the ACP ReadTextFileRequest the type
derived from permits extra properties, so the CLI kept passing both silently.
That is the argument for declaring the type standalone rather than Omit-ing
four of six inherited fields and quietly re-admitting the rest.
Also record the second behaviour delta of the detector merge in the design
doc: detectFileEncoding catches I/O errors and falls back to 'utf-8', where
detectFileHandleEncoding let them propagate. The failure is not lost — a handle
that fails the 8 KiB probe fails the streaming read immediately after — but a
different call now reports it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* feat(serve): page large text files by byte cursor
Line offsets address a byte stream, so `readText` resolves them by scanning
from byte 0. Paging a large log that way is O(n^2) across pages, and past
MAX_TEXT_SCAN_BYTES (8 MiB) a deep page is refused outright — agents had no
O(1) path short of dropping to GET /file/bytes and splitting lines themselves,
losing encoding handling, multibyte safety, and the binary_file refusal.
A response that leaves content behind now returns `hasMore`, and where a file
byte offset is derivable, an opaque `nextCursor`. Passing it back as `cursor`
resumes in O(1). Page 1 is an ordinary `limit` read, so clients never compute
byte offsets themselves, and a paging loop does not break when a file happens
to be small.
The cursor is unsigned base64url JSON carrying {off, size, dev, ino}, matching
encodeOrganizedCursor rather than the HMAC-signed transcript codec: the path is
re-resolved through the workspace boundary on every request, so a forged cursor
can only move the offset within a file the caller may already read — what
GET /file/bytes?offset= allows today. What the payload is for is staleness:
a replaced or truncated file yields hash_mismatch instead of bytes from the
wrong place, while an append leaves an outstanding cursor valid — the case the
feature exists for.
Every minted cursor points at the start of a line. When a single line exceeds
maxOutputBytes the reader emits a truncated prefix and skips to the next line
rather than resuming mid-line, because a mid-line cursor makes the following
page snap forward and silently drop the rest of that line at the seam. Windows
cut mid-line by a byte cap therefore report hasMore with no cursor, as do
non-UTF-8 snapshot reads whose decoded text is a UTF-8 re-encoding with no
mapping back to file offsets. That is why hasMore is a field rather than a
restatement of nextCursor.
Cursor reads branch before the size check, not by widening the window gate:
a cursor read of a file under MAX_READ_BYTES would otherwise land on the
snapshot path, which knows only line/limit, and silently return line 0.
Adds the workspace_file_read_cursor capability, per the convention that new
behavior gets a new tag, and retargets the scan-budget hint at cursor paging.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
* fix(core): advance UTF-8 cursors after truncation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* docs(serve): clarify cursor bootstrap limits
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* fix(sdk): raise daemon browser bundle budget
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
* test(serve): cover ACP cursor dispatch and cursor binary_file mapping (QwenLM#8002)
* fix(core): only set sawCrlf for emitted lines in cursor paging (QwenLM#8002)
---------
Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com>
|
Released in v0.21.2. |
…lpers (QwenLM#7967) * fix(serve): allow bounded reads of large text files * fix(serve): bound large-text reads by scan cost, not by which knob was set Follow-up to the bounded large-text read path. Three changes: Gate on any explicit window argument, not on `limit`. Gating on `limit` had the cost model backwards in both directions: `{ line: 900_000_000, limit: 20 }` was admitted despite walking the whole file, while `{ maxBytes: 4096 }` — satisfiable from the first 4 KiB — was refused. A read with no window argument at all still fails, since a caller that believes it holds the whole file may write it back truncated. Add MAX_TEXT_SCAN_BYTES (8 MiB). MAX_READ_BYTES caps what a read returns; nothing capped what it cost. Line offsets are resolved by scanning from byte 0, so a query param could turn into an uninterruptible multi-second scan of an arbitrarily large file — and on Windows hold a read handle for that span, blocking renames and deletes. Past the budget the read is refused with `file_too_large` pointing at readBytes, which reaches any offset in O(1). Tolerate appends on streamed windows. Requiring whole-file size/mtime stability after reading a prefix rejected reads whose returned bytes were still valid, and the case it rejected — tailing a live log — is the one this path exists for. Streamed windows now assert inode identity plus "did not shrink"; truncation and replacement are still rejected. Also: non-UTF-8 large text now returns `binary_file` rather than `file_too_large`, so a client retrying on 413 with a smaller window can't loop forever; and `readFileWithLineAndLimit` throws instead of silently ignoring a caller-supplied `fileHandle` on the by-path fallback. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(core): thread the descriptor instead of forking text-read helpers PR QwenLM#7947 pinned large-text reads to one inode by threading a caller-owned FileHandle into readTextRange as an optional field, plus a second field, forceStreaming, to suppress the buffering fast path. Two optional fields produced four combinations: one meaningful, one used by a single test, one unreachable, and — in readFileWithLineAndLimit — one that silently fell through to a by-path read, defeating the reason the caller opened a handle. Unify the two encoding detectors. detectFileEncoding now takes a path or a borrowed handle, so detectFileHandleEncoding is deleted along with the message discrepancy between them: an encoding iconv-lite cannot load now raises LargeNonUtf8TextError naming that encoding rather than deferring to the decoder's generic invalid-utf8 variant. Both still refuse the file, and the Serve boundary maps both to binary_file. Split the reader into readTextRange (path) and readTextRangeFromHandle (always streams, both byte bounds required). The unreachable combination and its untested readFileHandleBuffer are gone, and with no fileHandle parameter left for readFileWithLineAndLimit to ignore, the RangeError guarding that fallthrough is deleted too — the trap can no longer be expressed. CoreReadTextFileHandleRequest drops its required stats field. Nothing downstream read it, and because the ACP request type it extends permits extra properties, TypeScript accepted the dead argument silently. readFileHandleChunks becomes chunksFromHandle(fh, from) — the one seam byte-cursor text paging needs. No observable change at the Serve boundary: its 222 tests pass unmodified. Two fileSystemService tests were deleted rather than repaired; they asserted the arguments readFileWithLineAndLimit received, which is nothing once the handle path stops calling it. Their coverage lives in read-text-range.test.ts against real files and in workspace-file-system.test.ts at the real boundary. 258 production lines in core, net -71 overall. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * refactor(core): make CoreReadTextFileHandleRequest standalone Self-audit follow-up to f55c867. Two fields survived the reshape that the handle path never reads: - `stats` was documented as required ("must pass the Stats captured from that handle") and nothing downstream read it. The handle path always streams, so it never needs a size to choose a strategy, and the encoding probe does its own fstat. - `path` became dead once readTextRangeFromHandle replaced the path-plus-handle call. Errors are labelled with the path by the Serve boundary that owns it. Neither was caught by the compiler: the ACP ReadTextFileRequest the type derived from permits extra properties, so the CLI kept passing both silently. That is the argument for declaring the type standalone rather than Omit-ing four of six inherited fields and quietly re-admitting the rest. Also record the second behaviour delta of the detector merge in the design doc: detectFileEncoding catches I/O errors and falls back to 'utf-8', where detectFileHandleEncoding let them propagate. The failure is not lost — a handle that fails the 8 KiB probe fails the streaming read immediately after — but a different call now reports it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> * fix(serve): address review feedback on decoded-output cap, lineEnding, line validation, and docs (QwenLM#7967) * fix(serve): scope PR to descriptor threading, revert QwenLM#7947 behavior reversals (QwenLM#7967) Maintainer review adjudicated the pending design-reversal Critical: the descriptor-threading refactor is sound and boundary-neutral, but this PR also carried two undisclosed reversals of QwenLM#7947 that must not ride in on a refactor. Revert both, keeping only the refactor and the disclosed non-UTF-8 mapping: - Restore didFileVersionChange (size + mtime + ctime equality before and after the streamed read) in place of assertDidNotShrink. The size-only guard let a same-size in-place overwrite return torn content and a concurrent append run past the snapshot it opened against; the equality check rejects both with hash_mismatch. Main's three snapshot tests are restored unchanged. - Restore QwenLM#7947's limit-only large-text admission and drop MAX_TEXT_SCAN_BYTES / maxScanBytes / TextScanBudgetExceededError. The broadened wantsWindow admission and scan budget are a real contract change that belongs in its own PR against QwenLM#7947, not here. Kept: the descriptor-threading refactor (one detectFileEncoding, two entry points, readTextFileFromHandle/readTextRangeFromHandle, no mode flags) and the disclosed mapping of large non-UTF-8 windows to binary_file rather than file_too_large, which the maintainer explicitly approved. * test(core): port deep-read and encoding-probe tests to the handle path (QwenLM#7967) * test(core): pin handle read-to-EOF and unlimited-limit contracts (QwenLM#7967) * docs(serve): disclose handle-read behaviour deltas and add boundary tests (QwenLM#7967) Address maintainer re-verification (R2). The refactor's code is unchanged; this makes the documentation describe the actual diff and pins the remaining untested handle-path contracts. - Rewrite the design doc to match what the diff does against current main (one detector, two entry points, chunksFromHandle, standalone CoreReadTextFileHandleRequest) instead of the superseded intermediate state, and add a Behaviour deltas section disclosing the read-to-EOF chunk reader, the fresh-buffer-per-chunk change, the limit: Infinity admission, and the binary_file mapping for undecodable large text. - State the handle-layer omitted/infinite limit contract in qwen-serve-protocol.md, reconciled with the unchanged GET /file admission. - Add handle-bound abort tests (pre-aborted and mid-stream) to read-text-range.test.ts and an HTTP-layer test asserting a large non-UTF-8 file requested with a finite limit returns 422 binary_file. * test(core): add positive handle-read test for Infinity and omitted limit (QwenLM#7967) * fix(core): satisfy ReadTextRangeResult type in handle-read test mock (QwenLM#7967) * fix(core): bound handle reads to stat size and reject Infinity limit (QwenLM#7967) * fix(core): satisfy ReadTextRangeResult type in fileSystemService test mocks (QwenLM#7967) * refactor(core): reuse detectLineEndingFromContent in whole-file fast path (QwenLM#7967) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> * fix(core): export ReadTextRangeResult type and cover CRLF detection (QwenLM#7967) * test(core): cover whole-file lineEnding and truncatedByBytes shape (QwenLM#7967) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com> Co-authored-by: qwen-autofix[bot] <qwen-autofix[bot]@users.noreply.github.com> Co-authored-by: Qwen Code Bot <qwen-code-bot@users.noreply.github.com> Co-authored-by: qwen-code-ci-bot <qwen-code-ci-bot@users.noreply.github.com> Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com> Co-authored-by: qwen-code-dev-bot <qwen-code-dev@service.alibaba.com> Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
5 participants
What this PR does
This PR fixes Serve/ACP text reads for files larger than 256 KiB without
weakening the existing full-snapshot safety gate.
has a finite positive integer
limit. No-window, line-only, maxBytes-only,and line-plus-maxBytes requests still return
file_too_large.min(maxBytes ?? 256 KiB, 256 KiB), and the WorkspaceFileSystem boundaryvalidates
maxBytesin[1, 262144].StandardFileSystemService.readTextFileFromHandlemethodreuses Core's range reader with a caller-owned
FileHandle; it does notexpand the generic filesystem interface or ACP wire schema.
at the open-time size and reuse one 512 KiB buffer.
including both the open handle and the pathname. Concurrent append,
truncation, same-size rewrite (even with restored mtime), path replacement,
and symlink replacement are rejected before content is returned.
truncated: true, omit the full-file hash, and exposeoriginalLineCountonly when EOF made it exact. REST serializes an unknowncount as
null.binary_file; unsupported large non-UTF-8text remains
file_too_largewith a UTF-8 conversion hint.The existing 256 KiB full-snapshot paths used by unbounded reads, edit, hash,
and optimistic locking are unchanged. Small full-snapshot reads also now apply
maxBytesafter decoding, so UTF-16/32 expansion cannot exceed the returnedUTF-8 byte cap.
Why it's needed
Core gained streaming text-range reads in #6404, but Serve still rejected a
file above 256 KiB before applying its finite line window. As a result,
read_filerequests such as{ limit: 20 }failed withfile_too_largeandfell back to shell commands even though both the requested line count and
returned bytes were bounded.
The initial follow-up attempted to admit any explicit window and imposed an
8 MiB scan budget. That made deep line offsets unreachable without a cursor
and broadened the contract beyond this bug. This version keeps admission tied
to a finite
limitand removes the arbitrary scan cap while preserving boundedmemory and output.
Reviewer Test Plan
How to verify
{ limit: 20 }and{ line: 3, limit: 20 }; both shouldsucceed on the first read, return at most 256 KiB, report the complete
source size and
truncated: true, and omithash.each should retain the existing
file_too_largerefusal.rather than fail at an arbitrary scan budget.
and long CJK/emoji-line cases.
after a same-size rewrite, replace the pathname, and replace it with a
symlink. Expect
hash_mismatchorsymlink_escape, never mixed content.GET /fileroute carry the samebehavior; a partial REST response should omit
hashand serialize anunknown
originalLineCountasnull.The manual model-flow plan is recorded in
.qwen/e2e-tests/2026-07-28-serve-large-text-range-read.md.Evidence
git diff --check: passTested on
Risk & Scope
and returned content remain bounded; a future scan-cost policy should add a
cursor/continuation and request cancellation instead of silently making deep
offsets unreachable.
editing files above the full-snapshot cap remain out of scope.
Linked Issues
Fixes #7946
中文说明
本 PR 的改动
本 PR 在不放宽现有 256 KiB 全量快照安全门槛的前提下，修复 Serve/ACP
读取大型文本范围失败的问题。
limit时，大于 256 KiB 的 UTF-8 文本才进入流式范围读取。无窗口、仅 line、仅 maxBytes、line 加 maxBytes 仍返回
file_too_large。min(maxBytes ?? 256 KiB, 256 KiB)，WorkspaceFileSystem 同时校验maxBytes必须位于[1, 262144]。StandardFileSystemService.readTextFileFromHandle，复用现有范围读取器和调用方持有的 FileHandle，不修改通用文件系统接口和
ACP wire schema。
并复用一个 512 KiB 缓冲区。
pathname 的一致性。追加、截断、同大小覆盖（包括恢复 mtime）、路径替换和
符号链接替换都会在返回内容前被拒绝。
truncated: true、不返回全文件hash；只有扫描到 EOF 时才返回精确
originalLineCount，REST 对未知值固定返回
null。binary_file；不支持的大型非 UTF-8 文本仍映射为file_too_large，并提示转换为 UTF-8。无界读取、编辑、hash 和乐观锁仍走原有全量快照路径。小文件全量快照现在也会
在解码后执行
maxBytes限制，避免 UTF-16/32 解码扩张突破返回字节上限。为什么需要
Core 已在 #6404 支持流式文本范围读取，但 Serve 会在应用有限行窗口之前，
直接拒绝超过 256 KiB 的文件。因此
{ limit: 20 }这样的read_file请求虽然返回量有界，仍会收到
file_too_large并回退到 shell。此前的后续提交将任意显式窗口都视为准入条件，并增加了 8 MiB 扫描预算；
这会让没有 cursor 的深行偏移不可达，也扩大了本次修复的契约。本版本仅以
有限
limit解锁范围读取，移除任意扫描上限，同时保持内存和返回内容有界。验证证据
git diff --check：全部通过人工模型链路验证计划记录在
.qwen/e2e-tests/2026-07-28-serve-large-text-range-read.md。风险与范围
有界。未来如需扫描成本上限，应先增加 cursor/continuation 与请求取消，
而不是让合法深偏移静默变得不可达。
上限的文件编辑不在本次范围。
关联 Issue
Fixes #7946