Repository navigation
Replies: 1 comment
|
这条完全成立,而且根因、触发点、计数错误三处我都逐行核对过 —— 与你的描述一字不差。补充三样:为什么产品路径上必然命中、同缺陷的另外三个消费者、以及仓库里已有的修法先例。 1. 根因就在你指的那一行
const decoder = new TextDecoder() // utf-8, non-fatal: internal malformed bytes → U+FFFD(上一行 2. 触发点:有预算省略时前后缀各自独立 decode
const text = budgetOmitted > 0
? decoder.decode(keptPrefix) + decoder.decode(keptSuffix)
: decoder.decode(concat([prefix, suffix]))⇒ 后缀窗口的起点位于流中间,但解码器仍把它当作"解码起点" ⇒ 该处的 U+FEFF 被当作 BOM 剥掉。这正是你说的"window starts"。 3. 少报的机制
// Report omission against the bytes ACTUALLY returned…
const omitted = this.total - keptPrefix.length - keptSuffix.length
const truncated = omitted > 0计数基于裁剪后的原始字节,而 decoder 额外吞掉的 3 字节不进入这个减法 ⇒ 正是"omittedBytes 少报 3 字节"。 而 4. ⭐ 为什么产品路径上"必然"会命中
const retainer = new TextRetainer({ kind: 'head', maxBytes })它在 这一点建议写进报告的"影响面" —— 它把"一个 util 的边界条件"变成"每次 grep 都可能静默改字符"。 同缺陷还影响另外三处消费者:
5. ⭐ 修法与本仓既有做法一致(这条最有利于被采纳)全仓
⇒ 所以你的补丁不是"新增一种做法",而是"照仓库自己已有的写法补齐一处遗漏"。建议在报告里直接引这两个位置。 6. 你那条限定是对的,我确认一下你写"callers that need file-header BOM removal must apply that file-format policy before retention" —— 正确,而且现状下没有任何 caller 在 retainer 之前做文件头 BOM 策略 ⇒ 补丁不会破坏任何现有 BOM 语义。这句可以放心保留。 顺带排一个容易混淆的点(避免别人拿它反驳你): 7. 版本你标 8. 用户侧 workaround在走到 retainer 之前自己剥/记 BOM;或对含 U+FEFF 的行改用 |
Uh oh!
There was an error while loading. Please reload this page.
Summary
TextRetainercan silently remove a complete U+FEFF codepoint from retained text. In a truncated result, the reported exact omission count then understates the bytes absent from the returned text. In an untruncated result, content can disappear whiletruncatedremainsfalse.Independently reproduced against master at
d743267388641bc76f17c45ce8b4c231aed1d32c(0.2.1-alpha.2). Master was rechecked before posting and still points to that commit. The patch is available as a single fork commit; no upstream PR is opened, in accordance with the current contribution policy.Minimal reproduction
With Node 24.19.0, the following library-level example can be evaluated as ESM from the checkout root on the baseline above:
The U+FEFF is inside the original stream, not at its beginning. Its three UTF-8 bytes and the final
Zfit the four-byte tail window exactly.U+005AU+FEFF,U+005AomittedBytes{ kind: 'exact', count: 3 }{ kind: 'exact', count: 3 }truncatedtruetrueAll three strategies also return
Zfor a complete\uFEFFZinput that fits the budget, while reportingomittedBytes: { kind: 'none' }andtruncated: false.Root cause and observable impact
The shared decoder uses
new TextDecoder(). Its default BOM handling removes U+FEFF at a decode start. Node's TextDecoder documentation confirms thatignoreBOM: truepreserves the mark in decoded output.finish()decodes retained prefix/suffix windows independently when a middle gap exists, and decodes the complete buffer when nothing is omitted. A window start is not necessarily a file header. The omission count uses retained raw byte lengths after partial-codepoint trimming, so the decoder's additional three-byte deletion is not counted.This reaches a real product path:
previewLine()applies head retention to every displayed grep match, even when the line fits the budget. A real UTF-8 file containing the JavaScript string'ordinary line\n\uFEFFtarget\n', searched through thegreptool with{ pattern: 'target', path: 'feff.txt' }, returnsLine 2: targetrather thanLine 2: \uFEFFtarget. The search metadata loses the same character and does not report line truncation. The U+FEFF is on the second line, not a file-level BOM. The source file itself is not modified.The same helper is also used by job output retention and Session-reference retention. Those callers share the decoding behavior; the new assembled regression specifically covers grep.
Minimal patch
The production behavior change is confined to this decoder option. Existing partial UTF-8 cuts are still discarded, and malformed interior bytes retain non-fatal U+FFFD replacement. Callers that need file-header BOM removal must apply that file-format policy before retention.
cbbd6cd0b50a2118e5333579143dae546b865bb4dsh --profile headless, filesystem, ripgrep, tool pipeline and Session persistence; only the model response stream is authored/replayed.The commit changes nine files, including the bilingual README pair and pairing record. There are no public TypeScript API, package-export, dependency, configuration, or Session-format changes.
Verification
Node 24.19.0 and the repository-declared pnpm 11.28.5 were used, with frozen-lockfile installs. The following are actual local results, not a claim about remote CI:
pnpm exec vitest run packages/util/output-retention/tests/output-retention.spec.ts --coverage --coverage.include='packages/util/output-retention/src/index.ts'pnpm exec vitest run packages/fs/tool-fs-search/tests/integration.spec.ts packages/fs/tool-fs-search/tests/tools.spec.ts packages/context/session-reference/tests packages/jobs/tool-jobs/testspnpm run test:snapshot snapshots/session/headless.snapshot.ts -t fs-grep-feffpnpm run test:snapshot scripts/session-snapshot-corpus.corpus.tspnpm run test:docspnpm run lint:contracts-readypnpm run build:lib:clientpnpm run build:native-systemoutput-retention/lib/index.jssmoke\uFEFFZwith exactly three omitted bytes.pnpm run typecheckpassed, including the Host library build and Client contract typecheck.On the unpatched module, eight of the ten added library cases failed at the intended U+FEFF assertions; the two partial-codepoint controls passed. The real grep regression also failed before the fix. After generating the final POSIX Session fixture, temporarily reverting only the decoder option made the assembled replay fail solely because U+FEFF disappeared from tool-result text and search metadata. Restoring the option made that same read-only replay pass.
pnpm run doc-syncinitially finished with 41 passed and two failed on Windows. The CLI-help generator killed aweb --helpchild at its deadline during concurrent checks;pnpm run verify-cli-helppassed separately with all seven help pages current. The documentation-site test failure was WindowsEPERMwhen creating a file symlink; the identical seven-file documentation-site test command passed all 151 tests on Linux. The Windows doc-sync aggregate was not rerun and is not represented as an all-green run. No system symlink permissions were changed.Scope and limits
typert.host.jscontributors from the current Host build, with a locally built Linux native addon.Before posting, public Discussions, issues/PRs, commit messages and indexed fork code were searched for the relevant TextRetainer/BOM/U+FEFF/ignoreBOM terms. No equivalent earlier report or patch from another contributor was found within that search. #4407 concerns raw ripgrep stdout overflow, not this decoding/omission mismatch; the indexed fork match concerns raw-rg decoding rather than the shared retainer. This does not establish that nobody has discovered it privately or on an unindexed branch.
AI-assisted repository audit and implementation. The local repro and verification results above were captured during that work; the patch includes
Co-authored-by: Codex <noreply@openai.com>.All reactions