Repository navigation
fix(cli): read BOM-prefixed JSON files for timeline plans, grades and motion specs - #6845
Conversation
![heygengenesis[bot]](https://avatars.githubusercontent.com/in/2913576?s=60&v=4){kind=small}
There was a problem hiding this comment.
Verdict: REQUEST CHANGES · 1 blocker · 0 important · 0 nits · head fd836211
- blocker:
render --batch rows.jsonstill reads its file raw, atpackages/cli/src/commands/batchRender.ts:244(options.readFile ?? ((path) => readFileSync(path, "utf8"))), so a BOM-saved batch file still fails. I ranparseBatchRowson a\uFEFF[{"title":"a"}]file read the same way, and it throwsInvalid JSON in --batch: … Unexpected token '\uFEFF'. The PR body says "the other places that read a user-supplied JSON file use them", and this is one of those places. The fix isoptions.readFile ?? readJsonFileTextplus a BOM row test next to the other three.
Scope
Read all 8 files. Grepped packages/cli/src for JSON.parse(readFileSync and readFileSync(…, "utf8"). The rest read files the CLI writes itself (caches, state, manifests) or HTML. Ran jsonFlag, motionSpec and batchRender tests locally: 40/40 pass. grade-compare and a2Commands need workspace builds that I didn't do. The fails without the fix: line is present.
Edit accuracy: accurate 271 (base branch 271), smooth 112 of thoseThe gate passes. Quarantined, measured but not gated (0) |
fd83621 to
61fd537
Compare
61fd537 to
a6aeb6d
Compare
There was a problem hiding this comment.
Verdict: REQUEST CHANGES — 0 open, 1 new · 61fd537 → a6aeb6d
- blocker: CI › Fallow audit is red on this head in both runs. The one finding this PR introduces is
fallow/unused-exportondecodeJsonText(packages/cli/src/utils/jsonFlag.ts:11):batchRender.tswas its only importer, and the rebase moved it toreadJsonFileText. Dropping theexport(function decodeJsonText(bytes: Buffer): string {) clears the gate. Runningnpx [email protected] audit --base origin/mainlocally then gives✓ No issues in 13 changed files; the other 8 findings are inherited and don't gate.
The rebased code is the same as the head I approved, apart from the batch reader.
a6aeb6d to
69ce78a
Compare
fails without the fix:
reads an edit plan saved with a UTF-8 BOMinpackages/cli/src/timeline/a2Commands.ids.test.ts(also the grades, motion spec,reads a --batch file saved with a UTF-8 BOM,reads a policy saved with a UTF-8 BOManddetects a JSON transcript saved with a UTF-8 BOMtests)What
hyperframes timeline apply <plan> --planrefused a UTF-8 file with a BOM as "edit plan is not valid JSON", because it read the file as plain text andJSON.parserejects the leading BOM. Windows editors and PowerShell write that BOM.jsonFlag.tsalready decodes a BOM and UTF-16 for the JSON flags, so it now also exports the file and stdin readers, and the other places that read a user-supplied JSON file use them: thetimeline applyplan and thetimeline undoreceipt file, thegrade-comparegrades file, the motion spec, therender --batchrows file (now through this one reader instead of the copy #6605 added), the lambda policy file and JSON transcripts (detectFormat,normalize).lambda render-batchandcloudrunJSONL batches already survive a BOM (trim()drops it), so they are left alone. Files the CLI writes itself (caches, state) are untouched.What I measured
oxfmt --checkandoxlintclean.What I did NOT execute
timeline applyrun on a Windows-written file; the test writes the BOM bytes itself.Size
Lone fix nothing can carry: one shared reader and six call sites with a test each (batch already tested); it is a lone fix nothing can carry into another PR.