Skip to content

Patch returns stale diff and writes lose BOM after partial formatter failure #61415

Open

Description

@qkycir

What happened?

When a formatter writes corrections and then exits nonzero, Patch reports the pre-format diff even though the file on disk contains the corrections. A formatter that rewrites a UTF-8 BOM file without the BOM also leaves it missing after Patch or Write.

Formatter.file() returns false when no formatter succeeds. Both callers treat that as evidence that the file was unchanged: Patch uses its pre-format text, and FileWrite skips BOM synchronization. A nonzero exit code does not guarantee absence of writes.

Reproduction

On dev at 6d1c444, configure a formatter for .txt that runs:

require('fs').writeFileSync(process.argv.at(-1), 'FORMATTED\n'); process.exit(1)

Start with a UTF-8 BOM file containing before\n, then use Patch to replace before with created. The resulting file contains FORMATTED\n without its BOM, while output.files and metadata.files report +created. Write also loses the BOM with the same formatter.

Expected: Patch's diff reflects the final disk contents, and the original BOM is restored even if formatting exits unsuccessfully. This is a valid formatter outcome: RuboCop --autocorrect, for example, can correct some offenses while exiting 1 for remaining offenses.

Reproduced with the real Formatter service and a real Bun subprocess on Windows (Bun 1.4.4). Both regression tests fail on the current base.

Activity

  1. opencodejr commented on Nov 3, 2026

    @opencodejr

    Thanks for the detailed report. I reproduced this on dev at 6d1c444, which is also the latest dev.

    • A real .txt formatter that writes FORMATTED\n and then exits 1 changes the file, but Formatter.file() returns false.
    • Patch: on a UTF-8 BOM file containing before\n, replacing before with created reports -before / +created. The file on disk ends up as FORMATTED\n with no BOM.
    • Write: with the same formatter, the BOM is lost too.

    In both tools, false is taken to mean the file wasn't touched. Patch then reports its pre-format text and skips restoring the BOM, and Write returns before restoring it. The script below adds the regression tests to a checkout and runs them; the Patch and Write tests fail on current dev.

    Closely related, but not duplicates: #35071 and #23797 (keeping the BOM in file tools), #31422 (auto-formatting vs. apply_patch).

    Reproduction script (repro.sh)
    #!/usr/bin/env bash
    # Repro for #61415. Run from an opencode checkout (dev) after `bun install`:
    #   bash repro.sh /path/to/opencode
    # Appends three tests (real Formatter + Patch/Write tools) and runs them.
    set -e
    cd "${1:-.}"
    git apply <<'PATCH'
    diff --git a/packages/core/test/formatter.test.ts b/packages/core/test/formatter.test.ts
    index 8ceae1c3c..0d3596860 100644
    --- a/packages/core/test/formatter.test.ts
    +++ b/packages/core/test/formatter.test.ts
    @@ -374,3 +374,26 @@ describe("Formatter", () => {
         ),
       )
     })
    +
    +describe("repro #61415", () => {
    +  it.live("file() returns false after a formatter writes then exits 1", () =>
    +    withFormatter(
    +      {
    +        partial: {
    +          command: [process.execPath, "-e", "require('fs').writeFileSync(process.argv.at(-1), 'FORMATTED\\n'); process.exit(1)", "$FILE"],
    +          extensions: [".txt"],
    +        },
    +      },
    +      (formatter, directory) =>
    +        Effect.gen(function* () {
    +          const file = path.join(directory, "test.txt")
    +          yield* Effect.promise(() => fs.writeFile(file, "\uFEFFbefore\n"))
    +          const result = yield* formatter.file(file)
    +          const disk = yield* Effect.promise(() => fs.readFile(file, "utf8"))
    +          console.log("REPRO formatter.file() =", result, "disk =", JSON.stringify(disk))
    +          expect(result).toBe(false)
    +          expect(disk).toBe("FORMATTED\n")
    +        }),
    +    ),
    +  )
    +})
    diff --git a/packages/core/test/tool-patch.test.ts b/packages/core/test/tool-patch.test.ts
    index 3a571d2de..fcb188fac 100644
    --- a/packages/core/test/tool-patch.test.ts
    +++ b/packages/core/test/tool-patch.test.ts
    @@ -1267,3 +1267,31 @@ describe("PatchTool", () => {
         }),
       )
     })
    +
    +describe("repro #61415", () => {
    +  it.live("patch diff reflects disk and keeps BOM after partial formatter failure", () =>
    +    withTempTool((directory, registry) =>
    +      Effect.gen(function* () {
    +        const target = path.join(directory, "a.txt")
    +        yield* Effect.promise(() => fs.writeFile(target, "\uFEFFbefore\n"))
    +        // Mirrors real Formatter.file(): formatter rewrote the file, exited 1 -> returns false
    +        formatFile = (file) =>
    +          Effect.promise(async () => {
    +            await fs.writeFile(file, "FORMATTED\n")
    +            return false
    +          })
    +        const settled = yield* executeTool(
    +          registry,
    +          call("*** Begin Patch\n*** Update File: a.txt\n@@\n-before\n+created\n*** End Patch"),
    +        )
    +        expect(settled.status).toBe("completed")
    +        if (settled.status !== "completed") return
    +        const output = Schema.decodeUnknownSync(PatchTool.Output)(settled.output)
    +        const disk = yield* Effect.promise(() => fs.readFile(target, "utf8"))
    +        console.log("REPRO patch diff:\n" + output.files[0]?.patch + "\nREPRO disk =", JSON.stringify(disk))
    +        expect(output.files[0]?.patch).toContain("+FORMATTED")
    +        expect(disk).toBe("\uFEFFFORMATTED\n")
    +      }),
    +    ),
    +  )
    +})
    diff --git a/packages/core/test/tool-write.test.ts b/packages/core/test/tool-write.test.ts
    index 279c26c74..6b0434f64 100644
    --- a/packages/core/test/tool-write.test.ts
    +++ b/packages/core/test/tool-write.test.ts
    @@ -440,3 +440,29 @@ describe("WriteTool", () => {
         })
       })
     })
    +
    +describe("repro #61415", () => {
    +  it.live("write keeps BOM after partial formatter failure", () =>
    +    withTempDir((tmp) => {
    +      const fixture = makeWriteFixture()
    +      const target = path.join(tmp.path, "a.txt")
    +      fixture.formatFile = (file) =>
    +        Effect.promise(async () => {
    +          await fs.writeFile(file, "FORMATTED\n")
    +          return false
    +        })
    +      return Effect.promise(() => fs.writeFile(target, "\uFEFFbefore\n")).pipe(
    +        Effect.andThen(
    +          withTool(tmp.path, fixture, (registry) =>
    +            Effect.gen(function* () {
    +              yield* executeTool(registry, call({ path: "a.txt", content: "after\n" }, "call-repro"))
    +              const disk = yield* Effect.promise(() => fs.readFile(target, "utf8"))
    +              console.log("REPRO write disk =", JSON.stringify(disk))
    +              expect(disk).toBe("\uFEFFFORMATTED\n")
    +            }),
    +          ),
    +        ),
    +      )
    +    }),
    +  )
    +})
    PATCH
    cd packages/core
    bun test -t "repro #61415" test/formatter.test.ts test/tool-patch.test.ts test/tool-write.test.ts 2>&1 | grep -E "REPRO|^[-+]|\((pass|fail)\)" || true

    Run it: bash repro.sh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions