fix(cli): avoid duplicate ACP write BOM

[tt-a1i]

What this PR does

Updates AcpFileSystemService.writeTextFile() so ACP file writes preserve a UTF-8 BOM without duplicating it when the content already starts with the BOM marker.

Adds write-path tests for ACP forwarding, BOM insertion, existing BOM preservation, and fallback behavior when the ACP write capability is disabled.

Why it's needed

Before this change, the ACP adapter prepended \uFEFF whenever _meta.bom was true. If the content already started with \uFEFF, the adapter forwarded two BOM characters to the ACP client.

The local file system service already avoids this duplication when writing UTF-8 BOM files. This change keeps the ACP adapter consistent with that behavior.

Reviewer Test Plan

How to verify

Call AcpFileSystemService.writeTextFile() with _meta: { bom: true } and content that already starts with \uFEFF. Confirm the forwarded ACP writeTextFile request contains exactly one leading BOM marker.

Call it with _meta: { bom: true } and content without a BOM marker. Confirm the forwarded content gains one leading BOM marker.

Disable the ACP writeTextFile capability and confirm the fallback file system receives the original params unchanged.

Evidence (Before & After)

Before: _meta.bom: true plus content starting with \uFEFF produced forwarded content starting with two BOM markers.

After: npm test --workspace=packages/cli -- acp-integration/service/filesystem.test.ts passes with 8 tests, including existing-BOM preservation and missing-BOM insertion coverage.

After: npm test --workspace=packages/cli -- acp-integration/service acp-integration/acpAgent.test.ts passes with 2 test files and 141 tests. The run emits existing EventEmitter max-listener warnings from the ACP agent test harness but all tests pass.

After: npm run lint --workspace=packages/cli --if-present passes.

After: npx prettier --experimental-cli --check packages/cli/src/acp-integration/service/filesystem.ts packages/cli/src/acp-integration/service/filesystem.test.ts passes.

After: git diff --check passes.

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

Local macOS workspace with the repository npm dependencies installed.

Risk & Scope

• Main risk or tradeoff: This only changes the ACP write adapter's BOM normalization when _meta.bom is true; fallback and non-BOM writes are unchanged.
• Not validated / out of scope: Local Windows and Linux runs; non-UTF-8 encoding behavior is owned by the underlying file system implementation and is not changed here.
• Breaking changes / migration notes: No migration. Existing duplicated BOM content already written to disk is not rewritten by this PR.

Linked Issues

Fixes #5687

AI Assistance Disclosure

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

中文说明

What this PR does

更新 AcpFileSystemService.writeTextFile()，让 ACP 文件写入在保留 UTF-8 BOM 时，如果内容已经以 BOM marker 开头，不再重复添加 BOM。

新增 write path 测试，覆盖 ACP 转发、BOM 插入、已有 BOM 保留，以及 ACP write capability 关闭时的 fallback 行为。

Why it's needed

在这个改动之前，只要 _meta.bom 为 true，ACP adapter 就会 prepend \uFEFF。如果内容本身已经以 \uFEFF 开头，adapter 会向 ACP client 转发两个 BOM 字符。

本地 file system service 写 UTF-8 BOM 文件时已经避免了这种重复。本改动让 ACP adapter 与本地行为保持一致。

Reviewer Test Plan

How to verify

用 _meta: { bom: true } 且内容已经以 \uFEFF 开头调用 AcpFileSystemService.writeTextFile()。确认转发给 ACP writeTextFile 的请求只包含一个 leading BOM marker。

用 _meta: { bom: true } 且内容不带 BOM marker 调用它。确认转发内容会增加一个 leading BOM marker。

关闭 ACP writeTextFile capability，确认 fallback file system 收到的 params 保持原样。

Evidence (Before & After)

Before：_meta.bom: true 加上以 \uFEFF 开头的内容，会产生以两个 BOM marker 开头的转发内容。

After：npm test --workspace=packages/cli -- acp-integration/service/filesystem.test.ts 通过，8 个测试覆盖已有 BOM 保留和缺失 BOM 插入。

After：npm test --workspace=packages/cli -- acp-integration/service acp-integration/acpAgent.test.ts 通过，2 个测试文件共 141 个测试。运行中会出现 ACP agent 测试 harness 里已有的 EventEmitter max-listener warning，但所有测试通过。

After：npm run lint --workspace=packages/cli --if-present 通过。

After：npx prettier --experimental-cli --check packages/cli/src/acp-integration/service/filesystem.ts packages/cli/src/acp-integration/service/filesystem.test.ts 通过。

After：git diff --check 通过。

Tested on

OS	Status
🍏 macOS	✅ tested
🪟 Windows	⚠️ not tested
🐧 Linux	⚠️ not tested

Environment (optional)

本地 macOS 工作区，已安装仓库 npm 依赖。

Risk & Scope

• Main risk or tradeoff：仅在 _meta.bom 为 true 时调整 ACP write adapter 的 BOM normalization；fallback 和非 BOM 写入不变。
• Not validated / out of scope：未在本地跑 Windows 和 Linux；非 UTF-8 编码行为由底层 file system implementation 负责，本 PR 不改动。
• Breaking changes / migration notes：没有迁移逻辑。已经写入磁盘的重复 BOM 内容不会被本 PR 重写。

Linked Issues

Fixes #5687

AI Assistance Disclosure

我使用 Codex 来审查改动、对照现有模式做 sanity check，并帮助发现潜在边界情况。

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Thanks for the PR!

Template looks good ✓

On direction: This fixes a real bug — the ACP adapter was unconditionally prepending a BOM character even when the content already had one, producing double BOMs. That's a clear correctness issue in the ACP file write path, and aligning with the local file system service's existing BOM handling makes sense. Fixes #5687 as expected.

On approach: Minimal and focused — 4 lines changed in source, 110 lines of well-scoped tests. No scope creep, no drive-by refactors. The fix pattern (charCodeAt(0) !== 0xfeff) matches the idiom already used in fileSystemService.ts and several other BOM-aware spots in the codebase. Nothing to cut.

Moving on to code review. 🔍

中文说明

感谢贡献！

模板完整 ✓

方向：修复了一个真实 bug — ACP adapter 在内容已带 BOM 时仍无条件追加 BOM，导致双 BOM。这是 ACP 文件写入路径的正确性问题，与本地 file system service 的 BOM 处理保持一致是合理的。如预期修复了 #5687。

方案：最小且聚焦 — 源码改 4 行，测试 110 行，范围清晰。没有范围蔓延或顺手重构。修复模式（charCodeAt(0) !== 0xfeff）与 fileSystemService.ts 及代码库中其他 BOM 处理位置的惯例一致。无需裁剪。

进入代码审查 🔍

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Code Review

Independent proposal: Given "ACP adapter prepends BOM even when content already has one" — I'd add a guard to check if params.content already starts with U+FEFF before prepending. Same pattern the local fileSystemService.ts already uses.

PR's approach: Matches exactly. The condition params._meta?.['bom'] && params.content.charCodeAt(0) !== 0xfeff is the minimal correct fix. The BOM is only prepended when _meta.bom is requested and the content doesn't already start with one.

Reuse check: No new utility needed — charCodeAt(0) !== 0xfeff is the established one-liner used in fileSystemService.ts, commitAttribution.ts, notebook.ts, and the desktop files.ts/validation.ts/source-helpers.ts. Consistent.

No blockers found. The fix is correct, the tests cover all four paths (plain write, BOM requested with existing BOM, BOM requested without existing BOM, fallback when capability disabled), and the broader ACP test suite (502 tests across 18 files) passes cleanly.

Test Results

packages/cli $ npx vitest run src/acp-integration/service/filesystem.test.ts

 ✓ src/acp-integration/service/filesystem.test.ts (8 tests) 10ms

 Test Files  1 passed (1)
      Tests  8 passed (8)

packages/cli $ npx vitest run src/acp-integration/

 Test Files  18 passed (18)
      Tests  502 passed (502)

$ npx prettier --check packages/cli/src/acp-integration/service/filesystem.ts packages/cli/src/acp-integration/service/filesystem.test.ts

 All matched files use Prettier code style!

Before / After (tmux)

N/A — this is a non-user-visible internal fix in the ACP protocol's file write adapter. The bug (double BOM prepending) only manifests when an ACP client receives a writeTextFile request, not in any CLI output or TUI behavior. Unit tests above verify the fix.

中文说明

代码审查

独立方案： 已知"ACP adapter 在内容已带 BOM 时仍追加 BOM"——我会加一个 guard 检查 params.content 是否已以 U+FEFF 开头。与本地 fileSystemService.ts 使用的模式相同。

PR 方案： 完全一致。条件 params._meta?.['bom'] && params.content.charCodeAt(0) !== 0xfeff 是最小且正确的修复。仅在请求 _meta.bom 且内容不以 BOM 开头时才追加。

复用检查： 无需新工具 — charCodeAt(0) !== 0xfeff 是已有的惯用写法，在 fileSystemService.ts、commitAttribution.ts、notebook.ts、desktop 的 files.ts/validation.ts/source-helpers.ts 中均使用。一致。

无阻塞问题。 修复正确，测试覆盖四条路径（普通写入、请求 BOM 且已有 BOM、请求 BOM 且无 BOM、capability 关闭时的 fallback），完整 ACP 测试套件（18 个文件 502 个测试）全部通过。

测试结果

filesystem.test.ts: 8 passed
acp-integration/: 18 files, 502 passed
prettier --check: All matched files use Prettier code style!

Before / After（tmux）

N/A — 这是 ACP 协议文件写入 adapter 的内部修复，不影响 CLI 输出或 TUI 行为。Bug（双 BOM 追加）仅在 ACP client 收到 writeTextFile 请求时体现。上面的单元测试已验证修复。

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

Reflection

This is a clean, minimal bug fix that does exactly what it says. The double-BOM bug in the ACP write adapter is real — the old code unconditionally prepended \uFEFF whenever _meta.bom was true, ignoring that the content might already carry one. The fix is a single condition guard that matches the pattern already established in fileSystemService.ts and half a dozen other BOM-aware spots in the codebase.

My independent proposal (before reading the diff) was identical: add charCodeAt(0) !== 0xfeff to the condition. No simpler path exists — this is already the smallest possible correct fix.

The tests are thorough without being excessive: four new cases covering all four code paths through the method. The broader ACP suite (502 tests) is green. Formatting is clean.

Every line in the diff serves the stated goal. No drive-by refactors, no speculative additions, no scope creep. If I had to maintain this in six months, I'd thank the author for the focused change and clear test coverage.

Approving. ✅

中文说明

反思

这是一个干净、最小化的 bug 修复，完全如其所述。ACP 写入 adapter 的双 BOM bug 确实存在 — 旧代码在 _meta.bom 为 true 时无条件追加 \uFEFF，忽略了内容可能已带 BOM。修复是一个单一条件 guard，与 fileSystemService.ts 及代码库中多个 BOM 处理位置已有的模式一致。

我在阅读 diff 之前的独立方案完全相同：在条件中加入 charCodeAt(0) !== 0xfeff。不存在更简单的路径 — 这已经是最小且正确的修复。

测试充分而不过度：四个新用例覆盖方法的所有四条代码路径。完整 ACP 测试套件（502 个测试）全部通过。格式规范。

diff 中每一行都服务于既定目标。没有顺手重构、没有投机性添加、没有范围蔓延。如果六个月后要维护这段代码，我会感谢作者的聚焦改动和清晰的测试覆盖。

批准 ✅

— Qwen Code · qwen3.7-max

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

[wenshao]

✅ Local verification report (maintainer)

Built and tested this PR locally to confirm the fix before merge.

Environment

• macOS (Darwin 25.5), Node v22.22.2, npm 10.9.7
• PR head cb129bb43, base origin/main 2fd2104fa
• Method: applied the PR's exact two-file tree onto a clean origin/main checkout, then verified each claim independently.

Results

Check	Command	Result
Targeted tests	vitest run .../service/filesystem.test.ts	✅ 8/8 pass
ACP suite	vitest run .../acp-integration/service .../acpAgent.test.ts	✅ 128/128 pass (2 files)
Lint	eslint on both changed files	✅ clean
Types	tsc --noEmit (packages/cli)	✅ exit 0
Format	prettier --check on both files	✅ clean
Whitespace	git diff --check	✅ clean

Mutation test (the key evidence). To confirm the new tests actually guard the regression rather than passing vacuously, I reverted filesystem.ts back to the main version (the unconditional '' + content) while keeping the PR's new tests. Exactly one test failed — preserves a UTF-8 BOM without duplicating an existing marker:

- content: "Hello"        // expected: one BOM
+ content: "Hello"  // buggy main: two BOMs

The other 7 tests stayed green (they don't exercise the double-BOM path). This proves the new test pins the exact bug described in #5687.

Correctness. The guard params.content.charCodeAt(0) !== 0xfeff yields exactly one leading BOM in all normal cases and is consistent with the canonical normalization in packages/core/src/services/fileSystemService.ts (which strips an existing BOM before prepending). Because the ACP path forwards content as a string (not encoded bytes), conditional-prepend is the right shape here and produces output identical to the core reference. Edge cases check out: empty content + bom:true → just the BOM; content that is already a lone BOM → unchanged; bom:false → untouched via short-circuit.

Minor / non-blocking notes

1. The PR body says "141 tests"; the current suite is 128 (2 files). acpAgent.test.ts is unchanged since the PR's base, so 128 is the accurate count today — cosmetic only.
2. Optional: an explicit test for empty content + bom:true (the charCodeAt(0) → NaN path) would round out coverage. Behavior is already correct; not required for merge.

Verdict: LGTM. The fix is minimal, correct, mutation-verified, and consistent with the core reference. Safe to merge from my side.

🇨🇳 中文版

✅ 本地验证报告（维护者）

合并前在本地构建并测试了本 PR 以确认修复。

环境

• macOS（Darwin 25.5），Node v22.22.2，npm 10.9.7
• PR head cb129bb43，base origin/main 2fd2104fa
• 方法：把 PR 的两份文件原样应用到干净的 origin/main 检出上，再逐条独立验证 PR 的每个声明。

结果

检查项	命令	结果
目标测试	vitest run .../service/filesystem.test.ts	✅ 8/8 通过
ACP 套件	vitest run .../acp-integration/service .../acpAgent.test.ts	✅ 128/128 通过（2 个文件）
Lint	对两个改动文件跑 eslint	✅ 干净
类型	tsc --noEmit（packages/cli）	✅ exit 0
格式	对两个文件跑 prettier --check	✅ 干净
空白字符	git diff --check	✅ 干净

变异测试（关键证据）。 为确认新增测试是真的能挡住回归、而不是空过，我把 filesystem.ts 还原成 main 上的版本（无条件 '' + content），同时保留 PR 的新测试。恰好一个测试失败 —— preserves a UTF-8 BOM without duplicating an existing marker：

- content: "Hello"        // 期望：一个 BOM
+ content: "Hello"  // main 的 bug：两个 BOM

其余 7 个测试保持绿色（它们不会走到双 BOM 这条路径）。这证明新测试精确锁定了 #5687 描述的那个 bug。

正确性。 params.content.charCodeAt(0) !== 0xfeff 这个守卫在所有正常情况下都只产生一个前导 BOM，并且与 packages/core/src/services/fileSystemService.ts 中的规范化逻辑（先剥掉已有 BOM 再补）保持一致。由于 ACP 这条路径是把内容当作字符串转发（而非编码成字节），这里用「条件式补 BOM」是合适的形态，产出与 core 参考实现完全一致。边界情况也都没问题：空内容 + bom:true → 只有 BOM；内容本身就是单个 BOM → 保持不变；bom:false → 因短路而不动。

次要 / 非阻塞说明

1. PR 正文写的是「141 tests」，当前套件实际是 128（2 个文件）。acpAgent.test.ts 自 PR 的 base 起未变动，所以 128 是今天的准确数字 —— 仅是表述问题。
2. 可选：为空内容 + bom:true（即 charCodeAt(0) → NaN 路径）补一个显式测试可以让覆盖更完整。行为本身已经正确，合并不强制要求。

结论：LGTM。 修复最小化、正确、经变异测试验证，且与 core 参考实现一致。从我这边看可以安全合并。