Follow-ups from PR #2568 review: harden isBinaryBuffer, add direct util tests, fix MCP binary corruption

[dyoshikawa]

Background

PR #2568 fixed UTF-8 corruption of binary skill "other files" (e.g. images in a skill directory) by routing binary buffers through a raw-buffer read/compare/write path in DirFeatureProcessor.writeAiDirs. The review of that PR surfaced several non-blocking follow-ups that were intentionally not required for the merge. This issue tracks them.

Details

1. (mid) isBinaryBuffer misses binaries that are coincidentally valid UTF-8 — src/utils/file.ts. The current check treats any buffer that survives a UTF-8 round-trip as text. U+0000 is valid UTF-8, so a small binary that happens to be entirely valid UTF-8 (e.g. a .wasm file starting with \0asm) falls into the text path, where addTrailingNewline mutates its bytes (trims trailing whitespace, appends \n).
2. (low) No direct unit tests for the new utils — src/utils/file.test.ts has no direct cases for isBinaryBuffer or readFileBufferOrNull; they are only covered indirectly via dir-feature-processor.test.ts.
3. (low) isBinaryBuffer memory overhead — the round-trip allocates a full decoded string plus a second buffer copy per file (~3x transient memory for large assets). Acceptable at current scale, but a cheaper heuristic could avoid it.
4. (pre-existing) MCP channel still corrupts binary other files — src/mcp/skills.ts:44 converts other-file buffers via toString("utf-8") for MCP get/put, so binary skill files remain corrupted through the MCP surface. Same class of bug PR fix: write binary skill other-files without UTF-8 corruption #2568 fixed for import/generate.

Solution / Next Steps

• Harden the binary detection:

  export function isBinaryBuffer(buffer: Buffer): boolean {
    return buffer.includes(0) || !Buffer.from(buffer.toString("utf-8"), "utf-8").equals(buffer);
  }

• Add direct unit tests in src/utils/file.test.ts: CJK text buffer → not binary; JPEG header bytes (FF D8 FF E0 ...) → binary; NUL-containing buffer → binary; readFileBufferOrNull on a missing file → null, on an existing file → its bytes.

• Optionally replace the round-trip with a cheaper scan (e.g. NUL-byte check plus incremental UTF-8 validation) if profiling ever shows it matters.

• For the MCP surface, decide how binary other files should be represented in get/put (e.g. base64 with an encoding flag) instead of lossy toString("utf-8"), and apply the same binary/text split as PR fix: write binary skill other-files without UTF-8 corruption #2568.