fix(vault): stop vault_read rejecting non-ASCII text at the sniff boundary

[justintruong29]

What

vault_read rejects valid UTF-8 documents as binary when they contain non-ASCII text.

Why

The text-only gate has three layers; layer 3 sniffs the first vaultReadUTF8SniffBytes (8192) bytes and blocks the read when they are not valid UTF-8. That layer is needed — a PNG renamed notes.md with doc_type = note passes layers 1 and 2 — but the window was cut at a plain byte offset:

sniffLen := min(len(content), vaultReadUTF8SniffBytes)
if !utf8.Valid(content[:sniffLen]) { ... }

UTF-8 is variable width. Accented Vietnamese characters take 2–3 bytes, so byte 8192 regularly falls inside a character and the window ends with a dangling lead byte (C3 with its A2 cut away). utf8.Valid correctly reports that fragment as invalid, and the tool blames the file. The document itself is fine — the tool produced the broken bytes it then judged.

Two conditions must both hold, which is why it looks random: the file is larger than 8192 bytes, and the character at that offset is multi-byte. Sweeping the cut across 120 offsets of Vietnamese text blocked 29 of them — roughly one in four documents over 8 KB. Pure ASCII never triggers it, which is why the existing oversize test (30 KB of 'a') stayed green.

readCapped cut at max_bytes the same way. That does not block the read, but it puts a broken character at the end of every truncated non-ASCII document handed to the model.

How

trimPartialTrailingRune drops up to utf8.UTFMax-1 trailing bytes while the buffer is invalid, then stops. A rune of k bytes cut down to j bytes needs exactly j ≤ 3 bytes removed, so three attempts is a tight upper bound; still invalid after that means the damage is in the data, not at the cut.

Called at both cut sites, and only when the data was actually cut:

• Sniff: guarded by len(sniff) < len(content). A file that fits inside the window is judged untouched, so genuinely broken bytes at the end of a short file are still caught.
• readCapped: only on the len(buf) > maxBytes branch.

Binary content stays blocked — its invalid bytes are spread across the whole window, so trimming three bytes off the tail changes nothing.

Tests

• Rune straddling the sniff boundary → readable. Three sub-cases: 2-byte rune split, 3-byte rune split after one byte, and after two bytes.
• 20 KB of binary content named .md → still rejected.
• Broken trailing byte in a file below the sniff window → still rejected (covers the len(sniff) < len(content) guard; without it this file would be let through).
• max_bytes landing inside a rune → truncation marker present, no U+FFFD in the output.

Surface parity: Web UI / CLI / API contract N/A — internal to the vault_read tool. No request/response schema change, no new user-facing string (the existing error text is unchanged).

[clark-cant]

Review: PR #1501 — fix(vault): stop vault_read rejecting non-ASCII text at the sniff boundary

Summary: Fixes a UTF-8 boundary bug where vault_read falsely rejects valid non-ASCII documents as binary. The sniff window cuts at a fixed byte offset (8192), which can split a multi-byte rune — utf8.Valid then reports the fragment as invalid. Same bug in readCapped produces broken trailing characters.

Risk level: Low — targeted 2-file fix (+144/-3), no API changes.

Mandatory gates:

• Duplicate / prior implementation: ✅ clear — no prior PR/issue/commit addresses this specific boundary bug
• Project standards: ✅ clean Go, follows existing vault_read patterns
• Strategic necessity: ✅ clear value — fixes broken vault_read for all non-ASCII (Vietnamese, Chinese, etc.) documents >8KB

Findings:

• No critical or important issues
• trimPartialTrailingRune() is correct: trims up to utf8.UTFMax-1 (3) bytes from the tail, tight upper bound for any split rune
• Sniff guard len(sniff) < len(content) is the right condition — only trim when the window was actually truncated, preserving detection of genuinely broken short files
• readCapped fix is symmetric and correct
• Tests are excellent: 4 new test cases covering 2-byte/3-byte rune splits at boundary, binary content still rejected, short broken files still caught, max_bytes cut inside rune produces no U+FFFD
• PR body documents the root cause with evidence (120-offset sweep showing ~1/4 documents affected)

Verdict: Approve — Clean, well-tested fix for a real bug affecting non-ASCII vault documents.

Posted by github-maintain at 2026-08-03T17:28:15Z

[clark-cant]

Review: PR #1501 — fix(vault): stop vault_read rejecting non-ASCII text at the sniff boundary

Summary: Fixes a UTF-8 boundary bug where vault_read falsely rejects valid non-ASCII documents as binary. The sniff window cuts at a fixed byte offset (8192), which can split a multi-byte rune — utf8.Valid then reports the fragment as invalid. Same bug in readCapped produces broken trailing characters.

Risk level: Low — targeted 2-file fix (+144/-3), no API changes.

Mandatory gates:

• Duplicate / prior implementation: ✅ clear — no prior PR/issue/commit addresses this specific boundary bug
• Project standards: ✅ clean Go, follows existing vault_read patterns
• Strategic necessity: ✅ clear value — fixes broken vault_read for all non-ASCII (Vietnamese, Chinese, etc.) documents >8KB

Findings:

• No critical or important issues
• trimPartialTrailingRune() is correct: trims up to utf8.UTFMax-1 (3) bytes from the tail, tight upper bound for any split rune
• Sniff guard len(sniff) < len(content) is the right condition — only trim when the window was actually truncated, preserving detection of genuinely broken short files
• readCapped fix is symmetric and correct
• Tests are excellent: 4 new test cases covering 2-byte/3-byte rune splits at boundary, binary content still rejected, short broken files still caught, max_bytes cut inside rune produces no U+FFFD
• PR body documents the root cause with evidence (120-offset sweep showing ~1/4 documents affected)

Verdict: Approve — Clean, well-tested fix for a real bug affecting non-ASCII vault documents.

Posted by github-maintain at 2026-08-03T17:28:15Z