[StiensWout]

Editing a UTF-8 BOM file silently removed its marker on the first save. Keep the marker in workspace file reads so the editor writes it back. Let the shared lenient JSON parser accept a leading BOM so BOM-prefixed t3.json continues to parse.

Small fast-lane fix: localized decoding changes, no new state, interface or UI flow.

Proof

Found by the T3 Code bughunt; independently confirmed by Claude Opus 5.5 on main d899bddfa2a6f824411eaae10fb7a8fa0a37bbb7.

• Red: pnpm exec vp test run apps/server/src/workspace/WorkspaceFileSystem.test.ts -t 'reads UTF-8 files relative' fails on base because saving the edit removes EF BB BF.
• Green: the same test passes with the fix. Both focused files, WorkspaceFileSystem.test.ts and schemaJson.test.ts, pass. The JSON BOM case also fails against base source and passes with the fix.
• Real web proof: pnpm exec vp run scenarios:prove with the temporary file-editor-keeps-utf8-bom.e2e.ts scenario fails on base d899bddfa2 and passes on reviewed head 509abbd290. It opens the file, edits it in the real editor and checks the saved bytes. The scenario is proof-only and is not committed.
• pnpm exec vp run check:changed passes.
• Rebased before filing; git range-diff confirms the reviewed patch is unchanged.

Desktop and mobile were not independently exercised. This change is at their shared server read boundary. General encoding support is outside scope. The independent non-UTF-8 validation fix touches the same decoder and must retain both fatal: true and ignoreBOM: true when combined.

Local cubic found no issues; repository-specific context was unavailable. Trim found no changes needed. Opus's line-one review-context BOM nit is outside this save fix and remains a follow-up. Settings writers' existing normalization is also outside scope.

Council review

Reviewer	Round 1
Claude Opus 5.5	9/10
GPT 6.1 Sol	10/10

Small-PR council per Wout: two seats, no guest. No blocker or should findings remain.

Closes #36551

Created with GPT 6.1 Sol (lead, bughunt and review) in the Codex harness and Claude Opus 5.5 (independent verification, build, trim and review) in the Claude harness, through T3 Code.

[macroscopeapp]

Approvability

Verdict: Approved at b6b7abe

Macroscope's review found this PR approvable — Editor saves now preserve an existing UTF-8 BOM without adding duplicate markers. The change is confined to file-format preservation during requested saves, with existing workspace write protections unchanged.

^{You can add or adjust custom eligibility rules. Learn more.}

[coderabbitai]

No actionable comments were generated in the recent review.

Walkthrough

WorkspaceFileSystem.writeFile now preserves an existing UTF-8 BOM when replacement contents do not include one. A test checks that a read-edit-write cycle retains the BOM.

Changes: UTF-8 BOM handling

Layer / File(s)	Summary
Preserve BOM in workspace writes apps/server/src/workspace/WorkspaceFileSystem.ts, apps/server/src/workspace/WorkspaceFileSystem.test.ts	writeFile checks for a BOM on an existing file and prefixes BOM-less replacement contents when detected. The test checks that an edit preserves the BOM.

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: juliusmarminge, marcodotio

Merge Risk: Minimal · up to b6b7a

Saving an edited UTF-8 file now keeps its existing BOM, as the linked issue requests. The one remaining gap is a rare edge case: a file that is writable but not readable can still lose its BOM. No merge-blocking risk remains.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title names the server-side file editor save path and the UTF-8 BOM preservation, which is the main change in the changeset. It follows the conventional commit form and is specific enough for a te…
Description check		The description follows the template's main sections: Problem, Change, Scope, and Verification. It gives a clear problem statement, explains the server read and write change, lists focused red/green t…
Linked Issues check		[#36551] The requirement is that saving an edited UTF-8 BOM file keeps its leading EF BB BF bytes. At the reviewed head, WorkspaceFileSystem.writeFile (apps/server/src/workspace/WorkspaceFileSystem.ts…
Out of Scope Changes check		The changes in the whole-PR diff for apps/server/src/workspace connect to #36551. writeFile BOM preservation is the direct fix. The helper startsWithUtf8Bom and its O_NOFOLLOW and O_NONBLOCK flags sup…

Full details: Title check

Explanation

The title names the server-side file editor save path and the UTF-8 BOM preservation, which is the main change in the changeset. It follows the conventional commit form and is specific enough for a teammate scanning history.

Full details: Description check

Explanation

The description follows the template's main sections: Problem, Change, Scope, and Verification. It gives a clear problem statement, explains the server read and write change, lists focused red/green test evidence and a real web scenario, and states what was not checked. It also links issue #36551. The Scope section cites a linked issue and explains the bug-fix scope. The template's comment asks for a model and harness attribution, and the description includes one at the end. The description has no explicit Problem or Change headings, but the content covers both.

Full details: Linked Issues check

Explanation

[#36551] The requirement is that saving an edited UTF-8 BOM file keeps its leading EF BB BF bytes. At the reviewed head, WorkspaceFileSystem.writeFile (apps/server/src/workspace/WorkspaceFileSystem.ts) checks whether the existing target starts with U+FEFF. If the target has a BOM and the new contents do not, it prepends U+FEFF before NodeFSP.writeFile. The BOM check uses a nonblocking, no-follow read and returns false on open or read failure. The whole-PR diff against the merge base changes only writeFile and the BOM helper for this path. The added WorkspaceFileSystem.test.ts case reads bom.ts, replaces 42 with 43, calls writeFile, and compares the saved bytes with the original BOM-prefixed text. The write-side restore keeps the BOM whether or not readFile returns it, so the saved bytes match the expected behavior. The PR description says the test fails on base and passes at head. I did not run the tests or the real-editor scenario. The PR description also says the approach keeps the marker in reads, but the whole-PR diff does not change readFile. The implementation at head is write-side, so the description is out of date for this path.

Full details: Out of Scope Changes check

Explanation

The changes in the whole-PR diff for apps/server/src/workspace connect to #36551. writeFile BOM preservation is the direct fix. The helper startsWithUtf8Bom and its O_NOFOLLOW and O_NONBLOCK flags support the same save path, and the test case covers that path. No unrelated change appears in the inspected paths. Two items were not verified. The PR description and objectives mention a schemaJson.ts change for t3.json BOM parsing and a schemaJson.test.ts case. I did not locate that file in the diff, and my inspection of the packages path returned no changes. The raw summary also does not mention schemaJson. The objectives describe that parser change as secondary to the save bug. Its absence does not create an unrelated change in the inspected code. I did not inspect the full repository diff, so this verdict covers only the inspected paths.

Finishing Touches

Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Recent review info

Run configuration

• Source mode: Auto
• Repository: pingdotgg/t3code
• Branch: main
• File: .coderabbit.config.ts
• Commit: 96051c6
• Layers: Repository TS config
• Review profile: CHILL
• Plan: Advanced
• Run ID: d1a955b7-c3a0-4613-9ca2-90576558ec0d

Commits

Reviewing files that changed from the base of the PR and between 9312761 and b6b7abe.

Files selected for processing: 1

• apps/server/src/workspace/WorkspaceFileSystem.ts

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 0 remain after this review.

Dismissing prior approval to re-evaluate f3a74a8

[juliusmarminge]

Evidence

Real Linux web app, Chromium 156, 1280×800, light theme; fresh isolated data on each commit. Seed a UTF-8 BOM source file, open it in Files, change 42 to 43, let autosave finish, and inspect saved bytes in the app terminal. Also load a BOM-prefixed t3.json through the Actions menu.

Before	After
96051c630d	b6b7abee31
Saved bytes begin 65 78 70; BOM lost.	Saved bytes begin ef bb bf; BOM retained.
Before recording	After recording

Scenario source · Neutral terminal shell

scenarios:prove exited 0: failed on base because 657870 did not equal efbbbf, and passed on head.