test(tools): restore the drain guard and gate the TTY loop on the exit too

Two follow-ups from review.

The anti-redelivery poll my previous commit message described was not in the
tree. It was written, it passed, and then a `git checkout -- .` in the
falsification run that followed reverted it, because it was the one edit made
after the checkpoint commit. The message was amended afterwards and recorded work
that no longer existed. jatmn caught the mismatch. Restored here, and the claim
now matches the code: one more read after the served line, asserting it is not
repeated, which is the only guard for drain semantics on Windows since the
manager-level test that counts deliveries is skipped there.

The TTY loop stopped on the echo alone while asserting the exit as well. collect
can return the drained echo on a yield timer before markDone has run, so a poll
can carry got:hello with no exit_code meta yet and the assertion below would then
be about an exit nobody observed. It now waits for both. The echo term stays in
the condition deliberately: gating on exit_code alone would stop polling before a
slow shell start delivers the echo, which is the race this change exists to
remove.

internal/sandbox/profile.go

@@ -96,28 +96,15 @@ var protectedMetadataNames = []string{".git", ".zero", ".agents"}
 // gitMetadataWriteCarveouts below.
 var sandboxFullyProtectedMetadataNames = []string{".zero", ".agents"}
 
-// gitMetadataWriteCarveouts returns the .git paths that stay write-denied under
-// the OS-level sandbox even though the rest of .git is writable to git
-// subprocesses. Worktrees and submodules store .git as a regular pointer file,
-// so they protect that file itself: constructing .git/hooks below it makes
-// bwrap fail before the command starts. All other entry and error states retain
-// the legacy child paths. The pointer is deliberately not parsed here;
-// repository-controlled metadata must not redirect sandbox rules elsewhere.
+// gitMetadataWriteCarveouts returns the .git subpaths that stay write-denied
+// under the OS-level sandbox even though the rest of .git is writable to git
+// subprocesses. Nonexistent paths are harmless no-ops in every backend's
+// enforcement (seatbelt regex, bwrap ro-bind, Windows ACL deny entry).
 func gitMetadataWriteCarveouts(root string) []string {
-	return gitMetadataWriteCarveoutsWithLstat(root, os.Lstat)
-}
-
-func gitMetadataWriteCarveoutsWithLstat(root string, lstat func(string) (os.FileInfo, error)) []string {
-	gitPath := filepath.Join(root, ".git")
-	children := []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
+	return []string{
+		filepath.Join(root, ".git", "hooks"),
+		filepath.Join(root, ".git", "config"),
 	}
-	info, err := lstat(gitPath)
-	if err != nil || !info.Mode().IsRegular() {
-		return children
-	}
-	return []string{gitPath}
 }
 
 func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile {

internal/sandbox/profile_worktree_test.go

@@ -1,115 +0,0 @@
-package sandbox
-
-import (
-	"errors"
-	"os"
-	"path/filepath"
-	"testing"
-)
-
-func mustEqualCarveouts(t *testing.T, got, want []string) {
-	t.Helper()
-	if len(got) != len(want) {
-		t.Fatalf("carveouts = %v, want %v", got, want)
-	}
-	for i := range want {
-		if got[i] != want[i] {
-			t.Fatalf("carveout[%d] = %q, want %q", i, got[i], want[i])
-		}
-	}
-}
-
-// Worktree/submodule checkouts store .git as a pointer file. Protect the file
-// itself and never create a bwrap mount target beneath it.
-func TestGitMetadataWriteCarveoutsProtectsWorktreePointer(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	got := gitMetadataWriteCarveouts(root)
-	mustEqualCarveouts(t, got, []string{gitPath})
-	for _, carveout := range got {
-		if carveout == filepath.Join(gitPath, "hooks") || carveout == filepath.Join(gitPath, "config") {
-			t.Fatalf("carveout points beneath .git file: %q", carveout)
-		}
-	}
-}
-
-func TestLinuxBwrapPlanDoesNotMountBelowWorktreePointer(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := os.WriteFile(gitPath, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	profile := PermissionProfile{FileSystem: FileSystemPolicy{
-		Kind: FileSystemRestricted,
-		WriteRoots: []WritableRoot{{
-			Root:             root,
-			ReadOnlySubpaths: gitMetadataWriteCarveouts(root),
-		}},
-	}}
-	args := linuxBwrapFilesystemArgs(profile)
-	assertArgsContainSequence(t, args, "--ro-bind", gitPath, gitPath)
-	for _, child := range []string{"hooks", "config"} {
-		bogus := filepath.Join(gitPath, child)
-		if argsContainSequence(args, "--tmpfs", bogus) || argsContainSequence(args, "--ro-bind", bogus, bogus) {
-			t.Fatalf("bwrap args contain mount below .git file %q: %v", bogus, args)
-		}
-	}
-}
-
-func TestGitMetadataWriteCarveoutsPlainCheckout(t *testing.T) {
-	root := t.TempDir()
-	want := []string{
-		filepath.Join(root, ".git", "hooks"),
-		filepath.Join(root, ".git", "config"),
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-
-	if err := os.MkdirAll(filepath.Join(root, ".git"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-}
-
-func TestGitMetadataWriteCarveoutsPreservesSymlinkBehavior(t *testing.T) {
-	for _, targetKind := range []string{"directory", "file", "dangling"} {
-		t.Run(targetKind, func(t *testing.T) {
-			root := t.TempDir()
-			target := filepath.Join(root, "real.git")
-			switch targetKind {
-			case "directory":
-				if err := os.MkdirAll(target, 0o755); err != nil {
-					t.Fatal(err)
-				}
-			case "file":
-				if err := os.WriteFile(target, []byte("gitdir: ../outside\n"), 0o644); err != nil {
-					t.Fatal(err)
-				}
-			}
-			if err := os.Symlink(target, filepath.Join(root, ".git")); err != nil {
-				t.Skipf("symlink unavailable: %v", err)
-			}
-			want := []string{
-				filepath.Join(root, ".git", "hooks"),
-				filepath.Join(root, ".git", "config"),
-			}
-			mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), want)
-		})
-	}
-}
-
-func TestGitMetadataWriteCarveoutsPreservesInspectionErrorBehavior(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	want := []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
-	}
-	got := gitMetadataWriteCarveoutsWithLstat(root, func(string) (os.FileInfo, error) {
-		return nil, errors.New("inspection failed")
-	})
-	mustEqualCarveouts(t, got, want)
-}

internal/sandbox/profile_worktree_unix_test.go

@@ -1,21 +0,0 @@
-//go:build unix
-
-package sandbox
-
-import (
-	"path/filepath"
-	"syscall"
-	"testing"
-)
-
-func TestGitMetadataWriteCarveoutsPreservesSpecialGitEntryBehavior(t *testing.T) {
-	root := t.TempDir()
-	gitPath := filepath.Join(root, ".git")
-	if err := syscall.Mkfifo(gitPath, 0o644); err != nil {
-		t.Skip(err)
-	}
-	mustEqualCarveouts(t, gitMetadataWriteCarveouts(root), []string{
-		filepath.Join(gitPath, "hooks"),
-		filepath.Join(gitPath, "config"),
-	})
-}

internal/tools/bash_tool_test.go

@@ -70,6 +70,9 @@ func runBashToolHelper(command string) {
 		fmt.Println("listening", listener.Addr().String())
 		server := &http.Server{
 			Handler: http.HandlerFunc(func(response http.ResponseWriter, request *http.Request) {
+				// Printed per request, so a test can prove the session delivers output
+				// mid-flight: this line cannot exist before the first read has happened.
+				fmt.Println("served", request.URL.Path)
 				_, _ = response.Write([]byte("zero-server-ok"))
 			}),
 		}
@@ -522,7 +525,14 @@ func TestBashToolTimeoutKillsBackgroundChildren(t *testing.T) {
 	if result.Status != StatusError {
 		t.Fatalf("expected timeout error status, got %s: %q", result.Status, result.Output)
 	}
-	if elapsed > time.Second {
+	// The regression this guards is Run blocking until the background child's own
+	// sleep ends, so the bound only has to sit under that. One second sat almost
+	// exactly on it: the call legitimately costs a sandbox plan, a fork and exec of
+	// a shell plus a subshell, the 300ms timeout itself and the post-kill drain,
+	// and spawn alone was measured here at 720ms to 1.44s with every core busy. A
+	// merely slow runner therefore failed a correct system. Three seconds is still
+	// far below the child's lifetime, so nothing is given up.
+	if elapsed > 3*time.Second {
 		t.Fatalf("Run blocked %s past the 300ms timeout; background child held the pipes", elapsed)
 	}
 

internal/tools/edit_file.go

@@ -160,8 +160,7 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 	// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
 	// FileTracker re-baseline: recording pre-format content would make the very
 	// next edit look like an external modification and trip the conflict guard.
-	formatting := maybeFormatWrittenFile(ctx, absolutePath, updated)
-	updated = formatting.Content
+	updated = maybeFormatWrittenFile(ctx, absolutePath, updated)
 	// Re-baseline to the content we just wrote so subsequent edits in this session
 	// compare against the current on-disk state, not the pre-edit version.
 	newInfo, _ := os.Stat(absolutePath)
@@ -194,7 +193,6 @@ func (tool editFileTool) RunWithOptions(ctx context.Context, args map[string]any
 		suffix = "s"
 	}
 	summary := fmt.Sprintf("Successfully edited %s (replaced %d occurrence%s).", relativePath, replacedCount, suffix)
-	summary += formatting.notice(relativePath)
 	summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
 	result := okResult(summary)
 	result.ChangedFiles = []string{relativePath}

internal/tools/exec_command_test.go

@@ -3,6 +3,7 @@ package tools
 import (
 	"context"
 	"io"
+	"net"
 	"net/http"
 	"os"
 	"path/filepath"
@@ -373,10 +374,6 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 	if err != nil {
 		t.Fatalf("foreground server should return session_id, meta=%#v output=%q", start.Meta, start.Output)
 	}
-	addr := parseListeningAddress(start.Output)
-	if addr == "" {
-		t.Fatalf("server output did not include listening address: %q", start.Output)
-	}
 	t.Cleanup(func() {
 		writeTool.Run(context.Background(), map[string]any{
 			"session_id": sessionID,
@@ -384,7 +381,12 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 		})
 	})
 
-	response, err := http.Get("http://" + addr)
+	addr := waitForListeningAddress(t, writeTool, sessionID, start.Output)
+
+	// Bounded, so a server that accepts a connection and never answers fails here
+	// with its address instead of hanging until the package timeout kills the run.
+	client := &http.Client{Timeout: 30 * time.Second}
+	response, err := client.Get("http://" + addr)
 	if err != nil {
 		t.Fatalf("foreground exec server was not reachable at %s: %v; output=%q", addr, err, start.Output)
 	}
@@ -396,6 +398,153 @@ func TestExecCommandForegroundServerReturnsSessionAndServesHTTP(t *testing.T) {
 	if string(bytes) != "zero-server-ok" {
 		t.Fatalf("server response = %q", string(bytes))
 	}
+
+	// AND THE SESSION DELIVERS OUTPUT MID-FLIGHT, PINNED DETERMINISTICALLY.
+	//
+	// The helper prints this line only when it serves a request, so it cannot have
+	// been in the first read: the request had not happened yet. It can only arrive
+	// through a poll of a process that is still running, which is the delivery the
+	// address parse used to cover by accident and only on a machine slow enough to
+	// miss the first read. A regression where a live session buffers everything
+	// until it exits leaves the rest of this test green and fails here.
+	deadline := time.Now().Add(60 * time.Second)
+	var served strings.Builder
+	for !strings.Contains(served.String(), "served") && time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		served.WriteString(poll.Output)
+		if poll.Status != StatusOK {
+			t.Fatalf("the exec session was gone before it reported serving the request:\n%s", served.String())
+		}
+	}
+	if !strings.Contains(served.String(), "served") {
+		t.Fatalf("a still-running session never delivered the line the server printed while serving:\n%s", served.String())
+	}
+
+	// AND A READ DRAINS WHAT IT RETURNS.
+	//
+	// One more poll, because "the line showed up" is satisfied just as well by
+	// output that is re-delivered forever. drain copies and then nils the buffer,
+	// so this read carries nothing; if it stopped clearing, every session in
+	// production would repeat its whole output on every poll while every other
+	// assertion here stayed green. The manager-level test that asserts the same
+	// count is skipped on Windows, so this is the only place guarding it there.
+	again := writeTool.Run(context.Background(), map[string]any{
+		"session_id":    sessionID,
+		"chars":         "",
+		"yield_time_ms": 250,
+	})
+	if strings.Contains(again.Output, "served") {
+		t.Errorf("a second read returned output the first had already delivered, so reads are no longer draining:\n%s", again.Output)
+	}
+}
+
+// waitForListeningAddress polls the exec session until the helper says where it
+// is listening.
+//
+// ONE YIELD IS AN ALLOWANCE, NOT A GUARANTEE, AND THIS TEST TREATED IT AS ONE.
+//
+// The helper is this test binary re-executed. The first read used to be its only
+// chance: 500ms to be spawned through the process manager, reach net.Listen and
+// have its line drained back. A Windows CI runner under load does not get there
+// in time, so the read returned "Command is still running." with no address and
+// the test failed as though the server were broken. It failed that way on
+// unrelated contributor PRs, which is worse than a slow test: it reports somebody
+// else's change as the fault.
+//
+// Polling is what the banner in that very output tells the caller to do, so it is
+// what this does, up to a bound far above any plausible start.
+//
+// AND IT STILL FAILS FAST WHEN THE FAILURE IS REAL. A session that has gone away
+// answers with StatusError, and a process that exited before listening answers
+// with an exit_code line. Either is a genuine defect, so neither is worth waiting
+// out: they end the loop immediately with what the session actually said.
+//
+// Each read DRAINS: managedProcess.collect returns only the bytes produced since
+// the previous call, so these chunks are incremental and none of them is the whole
+// output. Each is parsed on its own, which is sound because the helper writes its
+// address with a single Println and a write that small does not tear. The whole
+// transcript is kept anyway, so a failure shows every chunk rather than the last.
+func waitForListeningAddress(t *testing.T, writeTool Tool, sessionID int, first string) string {
+	t.Helper()
+	if addr := validListeningAddress(first); addr != "" {
+		return addr
+	}
+	// AND WHEN IT DOES NOT, THIS TEST STOPS COVERING THE FIRST READ.
+	//
+	// That parse used to be the only assertion in the repo that a still-running
+	// session's first read carries the child's stdout at all: every other test
+	// either uses a tiny yield against a helper that prints later, or uses a large
+	// one against a helper that exits, which is the collect-on-exit branch. The
+	// manager-level equivalent is skipped on Windows because it shells out to
+	// /bin/sh. The coverage cannot be pinned deterministically without
+	// reintroducing the race, since collect() blocks the whole window either way,
+	// so it is given up deliberately here and paid back after the request below,
+	// where a line that cannot exist before the first read proves the poll path.
+	transcript := []string{first}
+	// Sixty seconds, not thirty. Starting this helper was measured here at 0.74s
+	// to 4.79s with every core saturated, on a box faster than a CI runner, and
+	// windows-latest additionally scans a freshly linked test binary on its first
+	// execution. A deadline of the same order as the latency it tolerates is the
+	// mistake that produced the 500ms budget; the point is a bound, and every
+	// genuine failure below leaves before reaching it anyway.
+	deadline := time.Now().Add(60 * time.Second)
+	for time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		transcript = append(transcript, poll.Output)
+		if addr := validListeningAddress(poll.Output); addr != "" {
+			return addr
+		}
+		// FAST-FAIL, BECAUSE THE LOOP HAS NO PACING OF ITS OWN. All the spacing
+		// comes from collect() waiting out its yield inside a LIVE session; every
+		// error return happens in microseconds. Continue removes the process the
+		// moment it observes the exit, so without these two the loop free-runs on
+		// a dead id: measured at 3028 polls in 5 seconds, which on a 4-vCPU runner
+		// would peg a core for the whole deadline and hand t.Fatalf hundreds of
+		// kilobytes of the same sentence. The exit result is also delivered
+		// exactly once, so folding it into the blob and continuing loses the one
+		// poll that says what went wrong.
+		if poll.Status != StatusOK {
+			t.Fatalf("the exec session was gone before the server reported an address:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+		}
+		if strings.Contains(poll.Output, "exit_code:") {
+			t.Fatalf("the server process exited before it reported an address:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+		}
+	}
+	t.Fatalf("the server never reported a listening address within the deadline:\n%s", strings.Join(transcript, "\n--- poll ---\n"))
+	return ""
+}
+
+// validListeningAddress is parseListeningAddress plus the check that what it
+// found is actually an address.
+//
+// A read DRAINS, and the formatter wraps each drain in its own body, so a line
+// split across two reads arrives as two fragments with banner text wedged between
+// them and no concatenation can rejoin it. The formatter also terminates the
+// partial chunk, so the fragment looks like a whole line and the bare parse
+// returns a truncated address: the test would then fail on an unreachable port
+// and point the next reader at the network instead of at the parse. Rejecting
+// anything that is not host:port keeps polling instead.
+func validListeningAddress(output string) string {
+	addr := parseListeningAddress(output)
+	if addr == "" {
+		return ""
+	}
+	host, port, err := net.SplitHostPort(addr)
+	if err != nil || host == "" {
+		return ""
+	}
+	if number, err := strconv.Atoi(port); err != nil || number <= 0 {
+		return ""
+	}
+	return addr
 }
 
 func parseListeningAddress(output string) string {
@@ -427,7 +576,12 @@ func TestExecCommandReapsFinishedUnpolledSession(t *testing.T) {
 		t.Fatalf("session_id is not numeric: %v", err)
 	}
 
-	deadline := time.Now().Add(2 * time.Second)
+	// Ten seconds, not two. Inside this the helper must cold-start, run its 250ms
+	// sleep, exit and pass the reap, and a cold start alone was measured at up to
+	// 1.44s under load: two seconds was a budget with almost nothing left in it. The
+	// loop returns as soon as the session is gone, so the bound only costs anything
+	// when the reap genuinely never happens.
+	deadline := time.Now().Add(10 * time.Second)
 	for {
 		if _, ok := manager.Snapshot(sessionID); !ok {
 			return
@@ -739,6 +893,13 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 		t.Fatalf("session_id is not numeric: %v", err)
 	}
 
+	// SAME SHAPE AS THE SERVER TEST ABOVE, one window narrower.
+	//
+	// The shell is started with a 10ms yield, so it has almost certainly not
+	// reached `read` yet, and the echo then had exactly one fixed 1000ms window to
+	// travel back through the PTY. Writing early is fine, the terminal buffers it,
+	// but a loaded runner can spend that whole second still getting the shell up.
+	// So the echo is polled for rather than demanded within one window.
 	result := writeTool.Run(context.Background(), map[string]any{
 		"session_id":    sessionID,
 		"chars":         "hello\n",
@@ -747,11 +908,39 @@ func TestExecCommandTTYSessionAcceptsInputOnLinux(t *testing.T) {
 	if result.Status != StatusOK {
 		t.Fatalf("write_stdin status = %s: %s", result.Status, result.Output)
 	}
-	if !strings.Contains(result.Output, "got:hello") {
-		t.Fatalf("expected PTY input output, got %q", result.Output)
+	var transcript strings.Builder
+	transcript.WriteString(result.Output)
+	exitCode := result.Meta["exit_code"]
+	// BOTH THE ECHO AND THE EXIT, because the test asserts both.
+	//
+	// collect can return the drained echo on a yield timer before markDone has
+	// run, so a poll can carry got:hello with no exit_code meta yet. Stopping on
+	// the echo alone would then assert an exit that had not been observed. The
+	// condition deliberately keeps the echo term as well: gating on exit_code
+	// alone would stop polling before a slow shell start delivers the echo, which
+	// is the race this whole change exists to remove.
+	deadline := time.Now().Add(30 * time.Second)
+	for (!strings.Contains(transcript.String(), "got:hello") || exitCode == "") && time.Now().Before(deadline) {
+		poll := writeTool.Run(context.Background(), map[string]any{
+			"session_id":    sessionID,
+			"chars":         "",
+			"yield_time_ms": 250,
+		})
+		transcript.WriteString(poll.Output)
+		if code := poll.Meta["exit_code"]; code != "" {
+			exitCode = code
+		}
+		if poll.Status != StatusOK {
+			// The session is gone. Either the echo already arrived, which the loop
+			// condition will see, or it never will and the assertions below say so.
+			break
+		}
 	}
-	if result.Meta["exit_code"] != "0" {
-		t.Fatalf("expected exited session, got meta=%#v output=%q", result.Meta, result.Output)
+	if !strings.Contains(transcript.String(), "got:hello") {
+		t.Fatalf("expected PTY input output, got %q", transcript.String())
+	}
+	if exitCode != "0" {
+		t.Fatalf("expected exited session, got exit_code=%q output=%q", exitCode, transcript.String())
 	}
 }
 

internal/tools/format_on_write.go

@@ -2,7 +2,6 @@ package tools
 
 import (
 	"context"
-	"errors"
 	"os"
 	"os/exec"
 	"path/filepath"
@@ -23,59 +22,8 @@ import (
 // the conflict guard.
 
 // formatOnWriteTimeout bounds one formatter run; a wedged formatter must never
-// hang a tool call. On timeout the unformatted write stands, and the caller
-// says so: see formatOnWriteResult.
-//
-// A var rather than a const so a test can shorten it. Nothing outside a test
-// assigns to it, and the deadline path was previously unreachable in a test at
-// any speed, which is part of why it went unnoticed that it reported nothing.
-var formatOnWriteTimeout = 10 * time.Second
-
-// formatOnWriteResult is the content after formatting, plus whether the
-// formatting that was supposed to happen actually did.
-//
-// A TIMEOUT IS NOT THE SAME KIND OF MISS AS THE OTHERS. Every other way this
-// falls back is a standing fact about the environment: the toggle is off, the
-// extension has no formatter, the binary is not installed. Those are silent on
-// purpose, because nothing is wrong and saying so on every write would be
-// noise. A deadline firing is different: formatting was configured, available
-// and expected, and the file was written unformatted anyway, on a machine that
-// was merely slow. Left silent, the caller believes it wrote canonical style
-// and finds out from a CI format check it cannot see, which is the thing this
-// feature exists to prevent.
-type formatOnWriteResult struct {
-	Content string
-	// Formatter is the binary that was run, named in the notice so the user can
-	// tell a slow gofmt from a slow prettier.
-	Formatter string
-	TimedOut  bool
-	// RestoreFailed means the file on disk is not known to hold Content.
-	//
-	// These formatters edit in place, so one that is killed or fails partway
-	// can leave the target truncated or half-rewritten: what a dead
-	// `prettier --write` leaves behind is not the input and not the output.
-	// Returning the written bytes while disk holds something else would put the
-	// tracker baseline, the diff preview and the file itself into three
-	// different states, so the failure paths write the bytes back. When even
-	// that fails the user has to hear about it: it is their file.
-	RestoreFailed bool
-}
-
-// notice is the line appended to the tool summary when formatting was expected
-// and did not happen, and empty in every other case.
-func (result formatOnWriteResult) notice(relativePath string) string {
-	if result.RestoreFailed {
-		return "\n\nWARNING: " + relativePath + " may not hold what was written. " +
-			result.Formatter + " was interrupted while rewriting it in place and the " +
-			"content could not be written back. Re-read the file before trusting it."
-	}
-	if !result.TimedOut {
-		return ""
-	}
-	return "\n\nNote: " + relativePath + " was written but not formatted: " +
-		result.Formatter + " did not finish within " + formatOnWriteTimeout.String() +
-		". The file holds exactly what was written, so a project format check may still flag it."
-}
+// hang a tool call. On timeout the unformatted write stands.
+const formatOnWriteTimeout = 10 * time.Second
 
 // formatterCommands maps a file extension to the formatter argv; the file path
 // is appended as the final argument. Only in-place, config-respecting,
@@ -121,20 +69,18 @@ func formatOnWriteEnabled() bool {
 // enabled and on PATH) and returns the file's content afterwards. Best-effort
 // throughout: any failure — no formatter, formatter error, timeout, unreadable
 // result — returns writtenContent so the caller's state matches the last write
-// it performed itself. Only the timeout is reported back, for the reason on
-// formatOnWriteResult.
-func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) formatOnWriteResult {
-	unformatted := formatOnWriteResult{Content: writtenContent}
+// it performed itself.
+func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenContent string) string {
 	if !formatOnWriteEnabled() {
-		return unformatted
+		return writtenContent
 	}
 	command, ok := formatterCommands[strings.ToLower(filepath.Ext(absolutePath))]
 	if !ok {
-		return unformatted
+		return writtenContent
 	}
 	binaryPath, err := exec.LookPath(command[0])
 	if err != nil {
-		return unformatted
+		return writtenContent
 	}
 	formatCtx, cancel := context.WithTimeout(ctx, formatOnWriteTimeout)
 	defer cancel()
@@ -143,35 +89,11 @@ func maybeFormatWrittenFile(ctx context.Context, absolutePath string, writtenCon
 	formatter.Dir = filepath.Dir(absolutePath)
 	formatter.Stdin = strings.NewReader("")
 	if err := formatter.Run(); err != nil {
-		unformatted.Formatter = command[0]
-		// THE FORMATTER EDITS IN PLACE, SO A FAILED RUN CAN LEAVE THE FILE
-		// NEITHER FORMATTED NOR AS WRITTEN. Killed by the deadline or by the
-		// caller, or exiting partway through its own rewrite, the target can hold
-		// a truncation. Returning the written bytes on top of that would leave the
-		// tracker baseline and the diff preview describing a file that is not on
-		// disk, which is a worse failure than the missing formatting: the next
-		// edit compares against content the file does not have.
-		//
-		// Written back unconditionally on this path rather than only when the
-		// bytes differ. Comparing first means reading the file to find out, and a
-		// read that fails leaves the same ambiguity this exists to remove.
-		if restoreErr := os.WriteFile(absolutePath, []byte(writtenContent), 0o644); restoreErr != nil {
-			unformatted.RestoreFailed = true
-		}
-		// OUR deadline, not the caller's cancellation and not the formatter's own
-		// exit status. A cancelled tool call is already being reported as
-		// cancelled, and a formatter that ran and refused the file usually means
-		// content it could not parse, which the write itself does not promise to
-		// fix. Neither is this notice's business; the restore above is, for all
-		// three.
-		if errors.Is(formatCtx.Err(), context.DeadlineExceeded) && ctx.Err() == nil {
-			unformatted.TimedOut = true
-		}
-		return unformatted
+		return writtenContent
 	}
 	formatted, err := os.ReadFile(absolutePath)
 	if err != nil {
-		return unformatted
+		return writtenContent
 	}
-	return formatOnWriteResult{Content: string(formatted), Formatter: command[0]}
+	return string(formatted)
 }

internal/tools/format_on_write_test.go

@@ -97,12 +97,9 @@ func TestFormatOnWriteFormatsAndKeepsTrackerConsistent(t *testing.T) {
 
 func TestFormatOnWriteSkipsUnknownExtensions(t *testing.T) {
 	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-	formatting := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw   text")
-	if formatting.Content != "raw   text" {
-		t.Fatalf("unknown extension must pass through: %q", formatting.Content)
-	}
-	if notice := formatting.notice("notes.xyz"); notice != "" {
-		t.Fatalf("an extension with no formatter is not a miss worth reporting, got %q", notice)
+	content := maybeFormatWrittenFile(context.Background(), filepath.Join(t.TempDir(), "notes.xyz"), "raw   text")
+	if content != "raw   text" {
+		t.Fatalf("unknown extension must pass through: %q", content)
 	}
 }
 
@@ -114,11 +111,8 @@ func TestFormatOnWriteFormatterLookupFailure(t *testing.T) {
 	if err := os.WriteFile(targetPath, []byte(uglyContent), 0o644); err != nil {
 		t.Fatal(err)
 	}
-	formatting := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
-	if formatting.Content != uglyContent {
-		t.Fatalf("missing formatter must return written content, got %q", formatting.Content)
-	}
-	if notice := formatting.notice("a.go"); notice != "" {
-		t.Fatalf("an uninstalled formatter is a standing fact, not a miss worth reporting, got %q", notice)
+	content := maybeFormatWrittenFile(context.Background(), targetPath, uglyContent)
+	if content != uglyContent {
+		t.Fatalf("missing formatter must return written content, got %q", content)
 	}
 }

internal/tools/format_on_write_timeout_test.go

@@ -1,280 +0,0 @@
-package tools
-
-import (
-	"context"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"runtime"
-	"strings"
-	"testing"
-	"time"
-)
-
-// A FORMATTER THAT RUNS OUT OF TIME MUST NOT LOOK LIKE ONE THAT SUCCEEDED.
-//
-// Every other way format-on-write falls back is a standing fact about the
-// environment: the toggle is off, the extension has no formatter, the binary is
-// not installed. Those are silent because nothing is wrong. A deadline firing is
-// different: formatting was configured, available and expected, and the file was
-// written unformatted anyway because the machine was slow. Silent, the caller
-// believes it wrote canonical style and learns otherwise from a CI format check
-// it cannot see, which is the failure this feature exists to prevent.
-//
-// THE DEADLINE IS SHRUNK RATHER THAN THE FORMATTER SLOWED. A formatter that
-// really sleeps has to be killed, and on Windows the batch file that hosts the
-// sleep leaves the sleeping grandchild holding the working directory, so the
-// test then fails in TempDir cleanup rather than on anything it asserts. An
-// expired budget against an ordinary fast formatter reaches the same branch by
-// the same route, deterministically and in microseconds.
-func TestFormatOnWriteReportsATimeout(t *testing.T) {
-	formatter := installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
-	shortenFormatOnWriteTimeout(t, time.Nanosecond)
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.fastfmt")
-	const written = "written   but   not   formatted\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	formatting := maybeFormatWrittenFile(context.Background(), target, written)
-
-	if formatting.Content != written {
-		t.Fatalf("content = %q, want the bytes that were written", formatting.Content)
-	}
-	if !formatting.TimedOut {
-		t.Fatal("a formatter cut off by the deadline was reported as an ordinary miss, so the write claims a formatting that never happened")
-	}
-	notice := formatting.notice("subject.fastfmt")
-	for _, want := range []string{"subject.fastfmt", "not formatted", formatter} {
-		if !strings.Contains(notice, want) {
-			t.Errorf("notice %q does not mention %q", notice, want)
-		}
-	}
-}
-
-// And the same formatter inside its budget says nothing at all, or the notice
-// above would be reporting the deadline rather than the miss.
-func TestFormatOnWriteStaysQuietWhenTheFormatterFinishes(t *testing.T) {
-	installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.fastfmt")
-	const written = "written\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	formatting := maybeFormatWrittenFile(context.Background(), target, written)
-	if formatting.TimedOut {
-		t.Error("a formatter that finished was reported as timed out")
-	}
-	if notice := formatting.notice("subject.fastfmt"); notice != "" {
-		t.Errorf("a successful format produced a notice: %q", notice)
-	}
-}
-
-// The caller cancelling is not this notice's business: that run is already
-// being reported as cancelled, and saying the formatter was too slow on top of
-// it would be wrong about why.
-func TestFormatOnWriteStaysQuietWhenTheCallerCancels(t *testing.T) {
-	installFakeFormatter(t, ".fastfmt", "fastfmt", succeedingFormatterScript())
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.fastfmt")
-	const written = "written\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-	formatting := maybeFormatWrittenFile(ctx, target, written)
-
-	if formatting.Content != written {
-		t.Fatalf("content = %q, want the bytes that were written", formatting.Content)
-	}
-	if formatting.TimedOut {
-		t.Error("a cancelled run was reported as a formatter timeout")
-	}
-	if notice := formatting.notice("subject.fastfmt"); notice != "" {
-		t.Errorf("a cancelled run produced a notice: %q", notice)
-	}
-}
-
-// A formatter that runs and refuses the file is also not a timeout. It usually
-// means content it could not parse, which the write does not promise to fix,
-// and reporting it as a slow machine would be wrong about the cause.
-func TestFormatOnWriteStaysQuietWhenTheFormatterFails(t *testing.T) {
-	installFakeFormatter(t, ".failfmt", "failfmt", failingFormatterScript())
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.failfmt")
-	const written = "unparseable\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	formatting := maybeFormatWrittenFile(context.Background(), target, written)
-	if formatting.TimedOut {
-		t.Error("a formatter that exited non-zero was reported as a timeout")
-	}
-	if notice := formatting.notice("subject.failfmt"); notice != "" {
-		t.Errorf("a failing formatter produced a timeout notice: %q", notice)
-	}
-}
-
-// installFakeFormatter puts a formatter on PATH and registers it in the command
-// table for the test's duration, returning the binary name the notice carries.
-func installFakeFormatter(t *testing.T, extension, name, script string) string {
-	t.Helper()
-	binaryName := name + formatterScriptExtension()
-	directory := t.TempDir()
-	if err := os.WriteFile(filepath.Join(directory, binaryName), []byte(script), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	t.Setenv("PATH", directory+string(os.PathListSeparator)+os.Getenv("PATH"))
-
-	previous, existed := formatterCommands[extension]
-	formatterCommands[extension] = []string{binaryName}
-	t.Cleanup(func() {
-		if existed {
-			formatterCommands[extension] = previous
-			return
-		}
-		delete(formatterCommands, extension)
-	})
-	return binaryName
-}
-
-func formatterScriptExtension() string {
-	if runtime.GOOS == "windows" {
-		return ".bat"
-	}
-	return ""
-}
-
-func succeedingFormatterScript() string {
-	if runtime.GOOS == "windows" {
-		return "@echo off\r\nexit /b 0\r\n"
-	}
-	return "#!/bin/sh\nexit 0\n"
-}
-
-func failingFormatterScript() string {
-	if runtime.GOOS == "windows" {
-		return "@echo off\r\nexit /b 3\r\n"
-	}
-	return "#!/bin/sh\nexit 3\n"
-}
-
-// shortenFormatOnWriteTimeout narrows the production deadline for one test.
-func shortenFormatOnWriteTimeout(t *testing.T, timeout time.Duration) {
-	t.Helper()
-	previous := formatOnWriteTimeout
-	formatOnWriteTimeout = timeout
-	t.Cleanup(func() { formatOnWriteTimeout = previous })
-}
-
-// A FAILED FORMATTER MUST NOT LEAVE THE FILE HALF-REWRITTEN.
-//
-// These commands edit in place, so one killed by the deadline, killed by the
-// caller, or exiting partway through its own rewrite can leave the target
-// truncated: neither the input nor the output. Returning the written bytes on
-// top of that would leave the tracker baseline and the diff preview describing
-// a file that is not on disk, and the next edit would compare against content
-// the file does not have.
-func TestFormatOnWriteRestoresTheFileWhenTheFormatterFails(t *testing.T) {
-	installFakeFormatter(t, ".clobberfmt", "clobberfmt", clobberingFormatterScript())
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.clobberfmt")
-	const written = "the bytes the caller wrote\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	requireFormatterClobbers(t, written)
-
-	formatting := maybeFormatWrittenFile(context.Background(), target, written)
-
-	onDisk, err := os.ReadFile(target)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if string(onDisk) != written {
-		t.Errorf("file on disk = %q, want the bytes that were written back", onDisk)
-	}
-	if formatting.Content != written {
-		t.Errorf("content = %q, want the bytes that were written", formatting.Content)
-	}
-	if formatting.RestoreFailed {
-		t.Error("restoration was reported as failed on a writable file")
-	}
-}
-
-// And the same for a run cut off by the deadline, which is the case the notice
-// already covers: the disclosure and the file have to agree.
-func TestFormatOnWriteRestoresTheFileOnTimeout(t *testing.T) {
-	installFakeFormatter(t, ".clobberfmt", "clobberfmt", clobberingFormatterScript())
-	shortenFormatOnWriteTimeout(t, time.Nanosecond)
-	t.Setenv("ZERO_FORMAT_ON_WRITE", "1")
-
-	target := filepath.Join(t.TempDir(), "subject.clobberfmt")
-	const written = "the bytes the caller wrote\n"
-	if err := os.WriteFile(target, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	requireFormatterClobbers(t, written)
-
-	formatting := maybeFormatWrittenFile(context.Background(), target, written)
-
-	onDisk, err := os.ReadFile(target)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if string(onDisk) != written {
-		t.Errorf("file on disk = %q, want the bytes that were written back", onDisk)
-	}
-	if !formatting.TimedOut {
-		t.Error("the deadline path stopped being reported once restoration was added")
-	}
-	if notice := formatting.notice("subject.clobberfmt"); !strings.Contains(notice, "not formatted") {
-		t.Errorf("notice = %q, want the timeout note", notice)
-	}
-}
-
-// clobberingFormatterScript truncates the file it is handed and then fails, the
-// way an interrupted in-place formatter leaves a partial rewrite.
-func clobberingFormatterScript() string {
-	if runtime.GOOS == "windows" {
-		return "@echo off\r\necho CLOBBERED> %1\r\nexit /b 3\r\n"
-	}
-	return "#!/bin/sh\necho CLOBBERED > \"$1\"\nexit 3\n"
-}
-
-// requireFormatterClobbers proves the fixture really does damage the file it is
-// handed, on a throwaway copy.
-//
-// It cannot be checked on the real target: restoration is the behaviour under
-// test, so when it works the evidence is gone, and asserting on the target
-// afterwards would either pass vacuously or report the opposite of what it saw.
-func requireFormatterClobbers(t *testing.T, written string) {
-	t.Helper()
-	probe := filepath.Join(t.TempDir(), "probe.clobberfmt")
-	if err := os.WriteFile(probe, []byte(written), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	binary, err := exec.LookPath("clobberfmt" + formatterScriptExtension())
-	if err != nil {
-		t.Fatalf("SETUP INVALID: the fake formatter is not on PATH: %v", err)
-	}
-	_ = exec.Command(binary, probe).Run()
-	after, err := os.ReadFile(probe)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if !strings.Contains(string(after), "CLOBBERED") {
-		t.Fatalf("SETUP INVALID: the fake formatter left %q, so it does not damage the file and restoration is not under test", after)
-	}
-}

internal/tools/write_file.go

@@ -114,8 +114,7 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
 	// Optional format-on-write (ZERO_FORMAT_ON_WRITE). Must run BEFORE the
 	// FileTracker baseline: recording pre-format content would make the very
 	// next edit look like an external modification and trip the conflict guard.
-	formatting := maybeFormatWrittenFile(ctx, absolutePath, content)
-	content = formatting.Content
+	content = maybeFormatWrittenFile(ctx, absolutePath, content)
 	// Baseline the freshly written content so a later edit/overwrite in this
 	// session compares against what is now on disk.
 	newInfo, _ := os.Stat(absolutePath)
@@ -138,7 +137,6 @@ func (tool writeFileTool) RunWithOptions(ctx context.Context, args map[string]an
 		lines++
 	}
 	summary := fmt.Sprintf("%s %s (%d lines).", verb, relativePath, lines)
-	summary += formatting.notice(relativePath)
 	summary += inlineDiagnostics(ctx, options, absolutePath, relativePath)
 	result := okResult(summary)
 	result.ChangedFiles = []string{relativePath}