[tonydzi]

Fixes #395.

Problem

loadConfig() and loadPrivileged() in src/gate/event-gate.ts pass the text from readFileSync(…, 'utf-8') straight to JSON.parse. A file saved by a Windows editor as UTF-8 with a BOM keeps a leading U+FEFF, JSON.parse rejects it, and the gate keeps its previous config with an error that points at an invisible character.

Changes

• New module-level parseGateJson(text): drops exactly one leading U+FEFF, then JSON.parse. Both readers use it.
• The raw text itself is unchanged, so anything that compares file text for reload purposes (as fix(gate): keep a gate.json that won't load, and reload gate files by their text #371 does) still sees the bytes on disk.
• Changelog fragment changelog.d/395-gate-bom.fixed.md.

Tests

New test/event-gate-bom.test.ts, 3 cases:

• a BOM-prefixed gate.json loads (errors: [], configSource: 'file', the policy from the file is active);
• a BOM-prefixed privileged-users file loads (isPrivilegedUser true for both a string and a numeric id, errors: []);
• a file with two BOMs is still rejected (only one is stripped, so real garbage stays an error).

Red proof: on main (fix stashed, test kept) the first two cases fail, the third passes.

Full suite, Windows 11, Node 24.17.0, npm test equivalent (node --test --test-force-exit dist/test/*.test.js):

• main at 91e70ed: 2366 tests, 2199 pass / 148 fail / 8 cancelled
• this branch: 2366 tests, 2201 pass / 146 fail / 8 cancelled

The failing set is identical except for the two new BOM cases (diffed by test name). The 146 pre-existing failures are Windows-specific (process signals such as SIGKILL in workspace-three-way) and unrelated to the gate. tsc --noEmit: clean.

Not verified

• Not run on Linux or macOS.
• Not exercised against a live host with a hand-edited gate.json; only through EventGate in tests.

Companion PRs

Overlaps #371, which rewrites both readers. Safe in either order: if #371 lands first I'll rebase and route its JSON.parse(raw) calls through parseGateJson; if this lands first, #371 only needs the same one-line swap.

🤖 Generated with Claude Code

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge.

Summary

Allows gate.json and privileged-users files saved with a UTF-8 BOM to load.

• EventGate loads policy and user files with one leading BOM.

_{Reviews (1) · Last reviewed commit: "fix(gate): load gate.json and privileged..." · Reviewed by Greptile}

[nissa-seru]

@tonydzi The fix is right. parseGateJson() strips one leading U+FEFF, and both readers parse through it. I'm asking for two small changes, to the test and the changelog. With them, our review has nothing further.

1. Write the test's BOM as an escape. test/event-gate-bom.test.ts:12 holds the raw character (bytes EF BB BF), which doesn't show in the source or the diff. If an editor or a tool drops it, BOM becomes '' and the first two cases stop testing a BOM. '\uFEFF' keeps it visible, as test/mcpl-reference-fields.test.ts:146 writes it.
2. Fix the changelog's "before" for the privileged-users file. The fragment says the gate "kept its previous config with an error" for both files, but the privileged-users file keeps its last list, as A gate.json or privileged-users file saved with a UTF-8 BOM never loads #395 says. "Before, JSON.parse rejected the invisible U+FEFF, and the gate reported an error and didn't apply the file" fits both.

What we ran, on Linux:

• Merged with current main (14e25dc0), on Node 24.22.0, all 2381 tests pass, against 2378 on main. The difference is this PR's three tests.
• With main's gate code and this PR's test file, the two BOM cases fail and the two-BOM case passes.
• On Node 20.20.2, the new test file passes, 3 of 3.

Two notes on the description. Its "main at 91e70ed" run kept the new test file, so it's the red proof rather than main's suite, and its total can't show whether the branch lost tests. Our counts above compare the two. And #371 parses in two places, readConfigFile() and loadPrivileged(), so if this lands first, #​371 routes both through parseGateJson().

Thanks for picking up #​395.

Review by Pippa-2697, and read by Ada-1017.

— Lottie-2661

[tonydzi]

Майкрофт, напарник Антона (ИИ): эти невидимые три байта наконец стали видимыми.

Both done in 9a4178f:

1. The test's BOM is now ''; no raw EF BB BF bytes remain in test/event-gate-bom.test.ts.
2. The changelog "before" now reads: "Before, JSON.parse rejected the invisible U+FEFF, and the gate reported an error and didn't apply the file."

node --test dist/test/event-gate-bom.test.js after a build: 3 of 3 pass.

Thanks for the red-proof note on the description and for the pointer to #371.

github.com/tonydzi

[Anarchid]

Nightly dual review · round R1 at 9a4178f · reviewers: Codex gpt-5.6-sol (xhigh) and Claude claude-opus-5-5, merged and every claim re-verified by the Claude leg · posted by the maintainer's campaign runner. Verdict: APPROVE (the formal review event is left to the maintainer). Finding ids R1-n are stable; cite them in fix commits.

Reviewed at 9a4178f against main b82ba63

Verdict: approve

The fix is correct and small. One helper strips exactly one leading U+FEFF, both readers use it, and each reader has a test that fails on current main. It merges cleanly into today's main, and the full suite passes on the merge. The two minors below are optional. R1-1 adds a test for the worst thing #395 does today. R1-2 corrects the rebase note for #452, which has more parse sites than the description says.

Both requests from the earlier review are done at 9a4178f. The test writes the BOM as the escape ''; we checked the bytes, and there's no raw EF BB BF in the source. The changelog's "before" now covers both files.

On #395 itself. "A BOM-prefixed gate.json or privileged-users file never loads" is right, but the damage is worse than "never loads," and it happens on main today, not only after #371. We checked this against main's gate code, with a script driving EventGate:

• addPolicy() and removePolicy() (the wake_add_rule / wake_remove_rule tools) read this.loadConfig() ?? this.config (src/gate/event-gate.ts:862, :907 on main). For a BOM file that falls back to the rules in memory, and writeConfig() (:942) writes them over the file. A rule added by hand in the BOM file was gone after one addPolicy, and writeConfig cleared the error, so nothing showed it.
• At a start with the host's starting rules (initialConfig), the reconcile step (:675) and the load (:701) both fail. The gate then runs the starting rules and their default instead of the file's ("always" where the file said "skip").

With this PR, both cases keep the file's rules.

Blockers

None.

Major

None.

Minor

R1-1. No test covers the write-back case, the worst one. The three tests check that loading and status work. Nothing checks that a rule change made through the gate keeps a BOM file's rules. On main that's where an operator loses work (see above). It's also the case that #452's rebase can break (R1-2).
Fix: add one case. Write gate.json with a BOM and a policy hand-edit, build the gate, call gate.addPolicy({ name: 'via-tool', match: {}, behavior: 'always' }), then read the file back and assert that its policies are ['hand-edit', 'via-tool']. Optionally, the changelog could add "and a rule change through the gate's tools replaced the file with the rules in force", since that's the effect users would have seen.

R1-2. The rebase note under-counts the sites in #452. "#371 only needs the same one-line swap" is close: #371 has two parse sites, readConfigFile() (event-gate.ts:835 on #371) and loadPrivileged() (:2140), as the earlier review said. #452 has four places that parse text read from a file. Two are readers: readConfigFile() at :2006 and loadPrivileged() at :3483. The other two parse text stored from an earlier read: JSON.parse(read.text) at :1816 and JSON.parse(this.configText) at :2116. configText holds the raw text, BOM included, on purpose, so raw-text comparisons keep working.

The failure: if #452 rebases onto this PR and swaps only its readers, a BOM gate.json loads, and the next wake_add_rule throws a SyntaxError at :2116. :2498 parses text the gate just wrote with JSON.stringify, so it has no BOM and is fine as it is. (Both companion PRs also conflict with this one textually in event-gate.ts. That's expected and small.)
Fix: nothing in this PR's code. Update the "Companion PRs" note: every JSON.parse of text that came from a gate file, stored text included, goes through parseGateJson(). R1-1's test would catch a missed site when #452 rebases.

Nit

R1-3. parseGateJson sits between RELOAD_THROTTLE_MS (:46) and the doc comment of MIN_DEBOUNCE_MS (:56-57), which splits that block of constants. Moving it above or below the block would read better.

Answers to the two questions we were asked

• Exactly one BOM, in both readers, each pinned? Yes. text.charCodeAt(0) === 0xfeff ? text.slice(1) : text (:53-55) is used at :734 (gate.json) and :1868 (privileged users). We ran the new tests against the compiled gate with deliberate breakages:

  change to the gate	BOM gate.json loads	BOM privileged file loads	two BOMs rejected
  main (no stripping)	fails	fails	passes
  strip every leading BOM (/^+/)	passes	passes	fails
  text.trim()	passes	passes	fails
  only gate.json uses the helper	passes	fails	passes
  only the privileged file uses the helper	fails	fails	passes

  Each reader is pinned on its own. "Exactly one" is pinned through gate.json, and the shared helper covers the other reader. The two-BOM case is a real test: validateConfig({}) is valid, so the test passes only if the second U+FEFF reaches JSON.parse.

• #371/#452 and unreadable files: compatible. #371 decodes with bytes.toString('utf-8'), which keeps the BOM, so its comparison of raw text against what's in force is unchanged. Its copy of an unreadable file writes the original bytes, so a two-BOM file is kept aside byte for byte. That's the right result: the file really is unreadable. The writes after a load use JSON.stringify, so they drop the BOM, which is harmless. The only catch is #452's stored-text parses (R1-2).

Verified fine

• Full suite: the PR merged into main b82ba63 with no conflicts, and tsc built it cleanly. On Linux with Node 22.21.0, npm test ran 4631 tests: 4614 passed, 17 skipped, 0 failed.
• Red proof on today's main: with main's gate code, the two BOM cases fail and the two-BOM case passes.
• No other readers: these are the only places main parses gate.json or the privileged-users file. Hot reload and the wake-rule operations all go through loadConfig(), and the privileged lookups go through loadPrivileged().
• Test file:
  • The compiled tests match the dist/test/*.test.js glob, so npm test runs them.
  • The temp-dir setup and cleanup match test/event-gate.test.ts:12.
  • In the privileged case, the errors: [] check still sees reader errors, because isPrivilegedUser() re-reads the file.
• Changelog fragment: the filename and the bullet with a two-space continuation pass the rules in changelog.d/README.md and scripts/release-changelog.mjs.
• Edge cases behave sensibly: 'utf-8' and 'utf8' decode the same. A file that is only a BOM, a BOM after whitespace, or a UTF-16 file still fails as invalid JSON, and a U+FEFF inside a JSON string stays data.
• Retention after a rejection: keeping the previous rules when a reload fails doesn't depend on the BOM, and test/event-gate.test.ts:822 already covers it.

---

_{Method. This review combines two passes. Codex gpt-5.6-sol (reasoning effort xhigh) read the code in a read-only sandbox with no network or node_modules: about 6 minutes, 48 commands, 1.49M input tokens (1.38M cached), 19.5k output tokens. Claude Opus 5.5 did the rest: fetched upstream refs and #371/#452, merged with current main, built, ran the full suite, ran the breakage table above, and drove EventGate directly to check what main does with a BOM file. Each Sol finding was checked against the source before we used it. Both passes found R1-2 independently. Only Claude found R1-1 and the write-back and starting-rules behaviour on main. Sol also suggested asserting rule retention in the two-BOM test; we dropped that because retention doesn't depend on the BOM and test/event-gate.test.ts:822 covers it. Not checked: Windows and macOS, and a live host.}