diff --git a/.github/scripts/check-mojibake.mjs b/.github/scripts/check-mojibake.mjs new file mode 100644 index 000000000..a1d9e6dfa --- /dev/null +++ b/.github/scripts/check-mojibake.mjs @@ -0,0 +1,96 @@ +// A file rewritten from a lossy decode carries U+FFFD where every character that could not +// be read used to be, and on disk that is the three bytes `EF BF BD` (#1188). Decoding the +// tree as UTF-8 to look for them does not work: on a clean `main` that reports 22 binary +// files and ~2,065,422 replacement characters (a GIF is not damaged text), so a gate built +// on `toString('utf8')` is red on `main` and no pull request can pass it. This gate reads +// bytes and calls a file binary when its first 8 KiB hold a NUL byte, which is what keeps +// `docs/images/*` and both `usage-sample.sqlite` copies out of the scan. +// +// The failure names the file, the occurrences and the lines, because the fix is "regenerate +// from the source", not "retype the Chinese". +// +// Usage: node .github/scripts/check-mojibake.mjs [--check] [root] +import { readdirSync, readFileSync } from 'node:fs' +import { join, sep } from 'node:path' + +export const REPLACEMENT = Buffer.from([0xef, 0xbf, 0xbd]) +export const SNIFF_BYTES = 8 * 1024 +// Installs, the git store, build output, and a test bench with its own homes. +const SKIP_DIRS = new Set(['node_modules', '.git', '.cache', 'coverage', 'dist', 'build']) +const SKIPPED_PREFIXES = ['.dsh-test-'] + +const toPosix = (path) => path.split(sep).join('/') + +// A NUL byte in the first 8 KiB is the one rule: text files in this repository are UTF-8 +// without NULs, and every binary asset (GIF, MP4, PNG, SQLite) has one early. +export function isBinary(buffer) { + const end = Math.min(buffer.length, SNIFF_BYTES) + for (let at = 0; at < end; at += 1) if (buffer[at] === 0) return true + return false +} + +// `occurrences` counts the triples, `lines` the distinct lines they sit on (a line can hold +// several). A binary file answers `undefined`: it is not text, so it is not damage. +export function scanBuffer(buffer) { + if (isBinary(buffer)) return undefined + let occurrences = 0 + let line = 1 + const lines = new Set() + for (let at = 0; at < buffer.length; at += 1) { + if (buffer[at] === 0x0a) { line += 1; continue } + if (buffer[at] === 0xef && buffer[at + 1] === 0xbf && buffer[at + 2] === 0xbd) { + occurrences += 1 + lines.add(line) + at += 2 + } + } + return { occurrences, lines: lines.size } +} + +// Every regular file under `root`, minus the skipped directories. Symlinks are not followed: +// a checkout carries no tracked one, and following one would leave the tree. +export function scanTree(root) { + const hits = [] + const skipped = [] + let text = 0 + const walk = (dir) => { + for (const entry of readdirSync(join(root, dir), { withFileTypes: true })) { + const rel = dir ? `${dir}/${entry.name}` : entry.name + if (entry.isDirectory()) { + if (SKIP_DIRS.has(entry.name) || SKIPPED_PREFIXES.some((prefix) => entry.name.startsWith(prefix))) continue + walk(rel) + continue + } + if (!entry.isFile()) continue + const result = scanBuffer(readFileSync(join(root, rel))) + if (result === undefined) { skipped.push(rel); continue } + text += 1 + if (result.occurrences > 0) hits.push({ path: rel, ...result }) + } + } + walk('') + return { hits, skipped: skipped.sort(), text } +} + +export function render(result) { + if (result.hits.length === 0) { + const names = result.skipped + const shown = names.length <= 6 ? names.join(', ') : `${names.slice(0, 3).join(', ')}, +${names.length - 3} more` + return `mojibake: ok — 0 occurrences of U+FFFD in ${result.text} text file(s) (${names.length} skipped as binary${names.length ? `: ${shown}` : ''})` + } + const total = result.hits.reduce((sum, hit) => sum + hit.occurrences, 0) + const lines = result.hits.reduce((sum, hit) => sum + hit.lines, 0) + return [ + `mojibake: FAIL — ${total} occurrence(s) of U+FFFD (EF BF BD) on ${lines} line(s) in ${result.hits.length} file(s):`, + ...result.hits.map((hit) => ` ${hit.path} — ${hit.occurrences} occurrence(s) on ${hit.lines} line(s)`), + 'Regenerate each file from its source (and its generator) instead of retyping the damaged text.', + ].join('\n') +} + +if (process.argv[1] === import.meta.filename) { + const args = process.argv.slice(2) + const root = args.find((arg) => !arg.startsWith('--')) ?? join(import.meta.dirname, '..', '..') + const result = scanTree(root) + console.log(render(result)) + if (result.hits.length > 0) process.exit(1) +} diff --git a/.github/scripts/check-mojibake.test.mjs b/.github/scripts/check-mojibake.test.mjs new file mode 100644 index 000000000..7c25f312c --- /dev/null +++ b/.github/scripts/check-mojibake.test.mjs @@ -0,0 +1,101 @@ +import assert from 'node:assert/strict' +import { spawnSync } from 'node:child_process' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { isBinary, render, scanBuffer, scanTree } from './check-mojibake.mjs' + +const ROOT = join(import.meta.dirname, '..', '..') +const SCRIPT = join(import.meta.dirname, 'check-mojibake.mjs') +const TRIPLE = Buffer.from([0xef, 0xbf, 0xbd]) +const GIF = 'docs/images/demo-tree.gif' +const SQLITE = 'core/test/fixtures/usage-sample.sqlite' + +const tempTree = (files) => { + const root = mkdtempSync(join(tmpdir(), 'check-mojibake-')) + for (const [rel, content] of Object.entries(files)) { + mkdirSync(join(root, rel, '..'), { recursive: true }) + writeFileSync(join(root, rel), content) + } + return root +} + +test('scanBuffer: one triple is one occurrence, a line holding two counts once', () => { + assert.deepEqual(scanBuffer(Buffer.from('a')), { occurrences: 0, lines: 0 }) + assert.deepEqual(scanBuffer(Buffer.from([0x61, ...TRIPLE, 0x62])), { occurrences: 1, lines: 1 }) + // The damaged i18n line from 9a033cef: two triples, on one line, inside a Chinese string. + const damaged = Buffer.concat([Buffer.from('x: \u8fde'), TRIPLE, TRIPLE, Buffer.from('\u5e94\u7528\n')]) + assert.deepEqual(scanBuffer(damaged), { occurrences: 2, lines: 1 }) + const twoLines = Buffer.concat([TRIPLE, Buffer.from('\n'), TRIPLE, Buffer.from('\n')]) + assert.deepEqual(scanBuffer(twoLines), { occurrences: 2, lines: 2 }) +}) + +test('isBinary: the repository assets are binary by content, a source file is not', () => { + const gif = readFileSync(join(ROOT, GIF)) + const sqlite = readFileSync(join(ROOT, SQLITE)) + assert.equal(isBinary(gif), true) + assert.equal(isBinary(sqlite), true) + assert.equal(isBinary(readFileSync(join(ROOT, 'core/src/client/i18n.js'))), false) + // Why the skip is the rule and not a nicety: decoded as UTF-8, this gif alone reports + // six figures of U+FFFD on a clean tree — the same false red the gate would have on main. + const naive = (gif.toString('utf8').match(/\uFFFD/g) ?? []).length + assert.ok(naive > 300_000, `the gif reads ${naive} replacement characters when decoded, so a decode-first scan is red on main`) + assert.deepEqual(scanBuffer(gif), undefined) +}) + +test('scanTree: the repository is clean and the binary assets are skipped by name', () => { + const result = scanTree(ROOT) + assert.deepEqual(result.hits, []) + assert.ok(result.text > 500, `scanned ${result.text} text files`) + // Tested, not assumed: both copies of the sqlite fixture and the images never enter the scan. + assert.ok(result.skipped.includes(SQLITE), `skipped: ${result.skipped.join(', ')}`) + assert.ok(result.skipped.includes(GIF)) + assert.ok(result.skipped.length >= 20, `skipped ${result.skipped.length} binary files`) +}) + +test('scanTree: a text file with a damaged line is named with its count, next to clean files', () => { + const root = tempTree({ + 'src/good.js': 'const ok = "\u8fde\u63a5\u5e94\u7528"\n', + 'src/bad.md': 'line one\n连\uFFFD\uFFFD应\u7528\nline three\n', + 'assets/pixel.bin': Buffer.from([0x00, 0x01, 0xef, 0xbf, 0xbd]), + }) + try { + const result = scanTree(root) + assert.deepEqual(result.hits, [{ path: 'src/bad.md', occurrences: 2, lines: 1 }]) + assert.deepEqual(result.skipped, ['assets/pixel.bin']) + assert.equal(result.text, 2) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test('render and the CLI: red names the file, occurrences and lines; clean answers ok', () => { + const root = tempTree({ + 'docs/note.md': 'one\n\u8fde\ufffd\ufffd\u5e94\u7528 and \ufffd\n', + 'img.bin': Buffer.from([0x00, 0xef, 0xbf, 0xbd]), + }) + try { + const red = spawnSync(process.execPath, [SCRIPT, '--check', root], { encoding: 'utf8' }) + assert.equal(red.status, 1) + assert.match(red.stdout, /3 occurrence\(s\) of U\+FFFD .* on 1 line\(s\) in 1 file\(s\)/u) + assert.match(red.stdout, /docs\/note.md — 3 occurrence\(s\) on 1 line\(s\)/u) + assert.match(red.stdout, /Regenerate each file from its source/u) + assert.doesNotMatch(red.stdout, /img\.bin —/u) + + writeFileSync(join(root, 'docs/note.md'), 'one\ntwo\n') + const green = spawnSync(process.execPath, [SCRIPT, '--check', root], { encoding: 'utf8' }) + assert.equal(green.status, 0, green.stdout) + assert.match(green.stdout, /mojibake: ok — 0 occurrences of U\+FFFD in 1 text file\(s\) \(1 skipped as binary: img\.bin\)/u) + } finally { + rmSync(root, { recursive: true, force: true }) + } +}) + +test('render: the binary skip is visible in the ok line, and a hit list ends with the remedy', () => { + const ok = render({ hits: [], skipped: ['a.gif', 'b.sqlite'], text: 3 }) + assert.match(ok, /0 occurrences of U\+FFFD in 3 text file\(s\) \(2 skipped as binary: a\.gif, b\.sqlite\)/u) + const fail = render({ hits: [{ path: 'x.js', occurrences: 4, lines: 2 }], skipped: [], text: 1 }) + assert.match(fail, /FAIL — 4 occurrence\(s\).* on 2 line\(s\) in 1 file\(s\):/u) + assert.match(fail, / x\.js — 4 occurrence\(s\) on 2 line\(s\)/u) +}) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 243af6f2d..77cb2b620 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -107,6 +107,8 @@ jobs: run: npx --yes -p typescript@5.9.3 tsc --noEmit --strict --skipLibCheck types/*.d.ts - name: Every test file and every workflow belongs to a layer (#749) run: node .github/scripts/testing-layers.mjs --check + - name: No text file carries a replacement character U+FFFD (#1188) + run: node .github/scripts/check-mojibake.mjs --check - name: The self-test catalog page matches docs/self-test.json (#782) run: node scripts/self-test-catalog.mjs --check - name: Tests of the repo scripts (risk tier, labels) diff --git a/AGENTS.md b/AGENTS.md index b6dac434b..8da16c366 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -256,6 +256,7 @@ The Bot team: 5. **`client.js` is generated.** Change `core/src/client/`, then run `npm run build` in `core/` and commit both files. Never edit `client.js` by hand. 6. **Do not touch the user's own DSH data.** The test bench keeps its data in `.dsh-test-/`. Never point a test at `~/.dsh` or at a DSH that the user runs. 7. **Reference-first.** An architecture change updates the matching subsystem doc (or a valid `Docs-not-needed:` trailer). Edit `.claude/skills/review-pr` only when the Verifier protocol itself changes. See [Reference-first](#reference-first). +8. **Read bytes, never re-decode.** A file rewritten from an outside source, or regenerated (`core/client.js`, `patches/ds-bot/*`, the `files/` mirrors), is read back as bytes and re-encoded by its generator — never decoded as UTF-8 from a fetch that may have split a multi-byte character, because `errors: 'replace'` turns the rest of that character into U+FFFD and the damage survives the build and the patch generation. The byte-level scan `node .github/scripts/check-mojibake.mjs --check` is the gate (a decode-first scan cannot be: it reports 22 binary files and ~2,065,422 replacement characters on a clean `main`, #1188); a red scan means "regenerate", not "retype the text". ## Commands diff --git a/changelog.d/1188-mojibake-gate.added.md b/changelog.d/1188-mojibake-gate.added.md new file mode 100644 index 000000000..43c5a9861 --- /dev/null +++ b/changelog.d/1188-mojibake-gate.added.md @@ -0,0 +1 @@ +A CI scan (`.github/scripts/check-mojibake.mjs`) now fails a pull request whose tracked text files contain U+FFFD — the three bytes `EF BF BD` a lossy decode writes in place of every character it could not read — and names each file with its occurrence and line counts, so the fix reads as "regenerate", not "retype the text". Binary assets are skipped by content (a NUL byte in the first 8 KiB), which is the rule that makes the gate usable at all: decoding the tree instead reports 22 binary files and ~2,065,422 replacement characters on a clean `main`. `AGENTS.md` now says why a file rewritten from an outside source, or regenerated, is re-read as bytes and re-encoded by its generator. See [#1188](https://github.com/ThinkFlowLab/FlyBot/issues/1188).