fix(acp): reject unsafe browser title text

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/specialist/exec.go

@@ -76,17 +76,12 @@ type BuildArgsInput struct {
 }
 
 type BuildResumeArgsInput struct {
-	SessionID    string
-	Prompt       string
-	CurrentDepth int
-	Manifest     Manifest
-	Cwd          string
-	// ParentModel and ParentReasoningEffort are the same fallbacks the fresh
-	// path takes, and they are here for the same reason: appendModelArgs uses
-	// them only when the manifest pins nothing of its own.
-	ParentModel           string
-	ParentReasoningEffort string
-	PermissionMode        string
+	SessionID      string
+	Prompt         string
+	CurrentDepth   int
+	Manifest       Manifest
+	Cwd            string
+	PermissionMode string
 }
 
 type BuildArgsResult struct {
@@ -349,18 +344,6 @@ func (executor Executor) BuildResumeArgs(input BuildResumeArgsInput) (BuildArgsR
 	}
 	args := []string{"exec", "--resume", sessionID}
 	args = append(args, promptArgs...)
-	// A RESUMED SPECIALIST KEEPS THE MODEL ITS MANIFEST PINNED.
-	//
-	// The fresh path appends this and the resume path did not, so a specialist
-	// pinned to a cheap model ran on that model once and then silently reverted
-	// to the parent's configured model on every resume. Nothing reports it: the
-	// child starts normally and the only symptom is the bill.
-	//
-	// Resuming does not restore the recorded model on its own. sessions.PrepareExec
-	// records the model it ran under but never feeds it back into provider
-	// construction, so without this the child takes whatever the config default
-	// resolves to now.
-	args = appendModelArgs(args, input.Manifest, input.ParentModel, input.ParentReasoningEffort)
 	args = append(args, "--auto", specialistAutonomy(input.PermissionMode), "--output-format", "stream-json")
 	// See BuildArgs: only plan/spec-draft propagate --permission-mode so --auto
 	// remains the authority for member/auto/ask/unsafe child resolution.
@@ -441,14 +424,12 @@ func (executor Executor) runResume(ctx context.Context, params TaskParameters, o
 		return ExecResult{}, err
 	}
 	built, err := executor.BuildResumeArgs(BuildResumeArgsInput{
-		SessionID:             params.Resume,
-		Prompt:                params.Prompt,
-		CurrentDepth:          options.CurrentDepth,
-		Manifest:              manifest,
-		Cwd:                   options.Cwd,
-		ParentModel:           options.ParentModel,
-		ParentReasoningEffort: options.ParentReasoningEffort,
-		PermissionMode:        options.PermissionMode,
+		SessionID:      params.Resume,
+		Prompt:         params.Prompt,
+		CurrentDepth:   options.CurrentDepth,
+		Manifest:       manifest,
+		Cwd:            options.Cwd,
+		PermissionMode: options.PermissionMode,
 	})
 	if err != nil {
 		return ExecResult{}, err

internal/specialist/resume_model_test.go

@@ -1,303 +0,0 @@
-package specialist
-
-import (
-	"context"
-	"strings"
-	"testing"
-
-	"github.com/Gitlawb/zero/internal/sessions"
-	"github.com/Gitlawb/zero/internal/streamjson"
-)
-
-// argValue returns the value following flag in argv, and whether it was present.
-func argValue(args []string, flag string) (string, bool) {
-	for index, arg := range args {
-		if arg == flag && index+1 < len(args) {
-			return args[index+1], true
-		}
-	}
-	return "", false
-}
-
-func pinnedManifest(model string, effort string) Manifest {
-	return Manifest{
-		Metadata: Metadata{
-			Name:            "skim",
-			Description:     "Bounded read-only lookups.",
-			Model:           model,
-			ReasoningEffort: effort,
-			Tools:           []string{"read_file"},
-		},
-		SystemPrompt:  "Find the thing and stop.",
-		ResolvedTools: []string{"read_file"},
-	}
-}
-
-// A RESUMED SPECIALIST KEEPS THE MODEL ITS MANIFEST PINNED.
-//
-// Metadata.Model exists so a bounded, delegated task can run on a cheaper model
-// than its parent. BuildArgs appended it; BuildResumeArgs did not. So the
-// specialist ran on the cheap model once, and the moment the orchestrator
-// resumed it the child fell back to whatever the parent's configured model
-// resolved to. Nothing surfaced it: the resumed child starts normally, does the
-// work, and the only symptom is the bill.
-//
-// Resuming does not restore the recorded model on its own, which is why the flag
-// has to be passed again rather than relied upon.
-func TestAResumedSpecialistKeepsItsPinnedModel(t *testing.T) {
-	manifest := pinnedManifest("claude-haiku-4.5", "")
-
-	fresh, err := (Executor{}).BuildArgs(BuildArgsInput{
-		Manifest:     manifest,
-		Prompt:       "find the thing",
-		CurrentDepth: 0,
-		ParentModel:  "claude-opus-4.1",
-	})
-	if err != nil {
-		t.Fatalf("BuildArgs: %v", err)
-	}
-	// SETUP: the fresh path really does pin it, or the comparison below is
-	// asserting against a path that never worked either.
-	freshModel, ok := argValue(fresh.Args, "--model")
-	if !ok || freshModel != "claude-haiku-4.5" {
-		t.Fatalf("SETUP INVALID: the fresh path passed --model %q (present=%t), want the manifest's model", freshModel, ok)
-	}
-
-	resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
-		SessionID:    "01HZZZZZZZZZZZZZZZZZZZZZZZ",
-		Prompt:       "keep going",
-		CurrentDepth: 0,
-		Manifest:     manifest,
-		ParentModel:  "claude-opus-4.1",
-	})
-	if err != nil {
-		t.Fatalf("BuildResumeArgs: %v", err)
-	}
-	resumedModel, ok := argValue(resumed.Args, "--model")
-	if !ok {
-		t.Fatalf("a resumed specialist carries no --model at all, so it reverts to the parent's configured model: %v", resumed.Args)
-	}
-	if resumedModel != "claude-haiku-4.5" {
-		t.Fatalf("a resumed specialist runs on %q, want the manifest's pinned %q", resumedModel, "claude-haiku-4.5")
-	}
-}
-
-// And a manifest that pins nothing still inherits the parent's model, which is
-// what makes the fallback in appendModelArgs meaningful rather than the pinned
-// case being special-cased.
-func TestAResumedSpecialistWithNoPinInheritsTheParentModel(t *testing.T) {
-	resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
-		SessionID:    "01HZZZZZZZZZZZZZZZZZZZZZZZ",
-		Prompt:       "keep going",
-		CurrentDepth: 0,
-		Manifest:     pinnedManifest("", ""),
-		ParentModel:  "claude-opus-4.1",
-	})
-	if err != nil {
-		t.Fatalf("BuildResumeArgs: %v", err)
-	}
-	model, ok := argValue(resumed.Args, "--model")
-	if !ok || model != "claude-opus-4.1" {
-		t.Fatalf("a resumed specialist with no pinned model passed --model %q (present=%t), want the parent's", model, ok)
-	}
-}
-
-// The reasoning-effort rule travels with the model, or the two paths disagree
-// about what a pinned model implies. appendModelArgs inherits the parent's
-// effort ONLY when the manifest pins no model of its own: a manifest that chose
-// a different model has not agreed to the parent's effort for it.
-func TestAResumedSpecialistFollowsTheSameReasoningEffortRule(t *testing.T) {
-	t.Run("pinned model does not inherit parent effort", func(t *testing.T) {
-		resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
-			SessionID:             "01HZZZZZZZZZZZZZZZZZZZZZZZ",
-			Prompt:                "keep going",
-			CurrentDepth:          0,
-			Manifest:              pinnedManifest("claude-haiku-4.5", ""),
-			ParentModel:           "claude-opus-4.1",
-			ParentReasoningEffort: "high",
-		})
-		if err != nil {
-			t.Fatalf("BuildResumeArgs: %v", err)
-		}
-		if effort, ok := argValue(resumed.Args, "--reasoning-effort"); ok {
-			t.Fatalf("a manifest that pinned its own model inherited the parent's effort %q", effort)
-		}
-	})
-
-	t.Run("no pinned model inherits parent effort", func(t *testing.T) {
-		resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
-			SessionID:             "01HZZZZZZZZZZZZZZZZZZZZZZZ",
-			Prompt:                "keep going",
-			CurrentDepth:          0,
-			Manifest:              pinnedManifest("", ""),
-			ParentModel:           "claude-opus-4.1",
-			ParentReasoningEffort: "high",
-		})
-		if err != nil {
-			t.Fatalf("BuildResumeArgs: %v", err)
-		}
-		effort, ok := argValue(resumed.Args, "--reasoning-effort")
-		if !ok || effort != "high" {
-			t.Fatalf("a manifest pinning nothing passed --reasoning-effort %q (present=%t), want the parent's", effort, ok)
-		}
-	})
-}
-
-// The two builders agree on where the flag sits relative to the rest of the
-// argv, so a future change to one does not silently reorder only the other.
-func TestBothBuildersPlaceTheModelBeforeTheAutonomyFlag(t *testing.T) {
-	manifest := pinnedManifest("claude-haiku-4.5", "")
-	fresh, err := (Executor{}).BuildArgs(BuildArgsInput{Manifest: manifest, Prompt: "go", CurrentDepth: 0})
-	if err != nil {
-		t.Fatalf("BuildArgs: %v", err)
-	}
-	resumed, err := (Executor{}).BuildResumeArgs(BuildResumeArgsInput{
-		SessionID: "01HZZZZZZZZZZZZZZZZZZZZZZZ", Prompt: "go", CurrentDepth: 0, Manifest: manifest,
-	})
-	if err != nil {
-		t.Fatalf("BuildResumeArgs: %v", err)
-	}
-	for _, argv := range [][]string{fresh.Args, resumed.Args} {
-		model := indexOf(argv, "--model")
-		auto := indexOf(argv, "--auto")
-		if model < 0 || auto < 0 || model > auto {
-			t.Fatalf("--model at %d, --auto at %d in %s", model, auto, strings.Join(argv, " "))
-		}
-	}
-}
-
-func indexOf(args []string, want string) int {
-	for index, arg := range args {
-		if arg == want {
-			return index
-		}
-	}
-	return -1
-}
-
-// AND THE CALL SITE PASSES IT, WHICH THE BUILDER TESTS ABOVE CANNOT SEE.
-//
-// Every test above calls BuildResumeArgs directly. Dropping the ParentModel
-// field from the runResume call site still compiles and still passes all of
-// them, because the builder is doing its job with whatever it is handed. The
-// defect this fix repairs lived at the call site, not in the builder, so it
-// needs a test that goes through Run.
-//
-// Driven through the real Run dispatch with the RunChild seam capturing argv.
-func TestRunResumePassesTheParentModelThroughToTheChild(t *testing.T) {
-	store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
-	parent, err := store.Create(sessions.CreateInput{SessionID: "parent_session"})
-	if err != nil {
-		t.Fatalf("create parent: %v", err)
-	}
-	if _, err := store.Create(sessions.CreateInput{
-		SessionID:       "child_task",
-		SessionKind:     sessions.SessionKindChild,
-		Tag:             SessionTagSpecialist,
-		Depth:           1,
-		ParentSessionID: parent.SessionID,
-		AgentName:       "skim",
-		TaskID:          "child_task",
-	}); err != nil {
-		t.Fatalf("create child: %v", err)
-	}
-
-	zero := 0
-	var captured []string
-	executor := Executor{
-		BinaryPath:   "/usr/local/bin/zero",
-		SessionStore: store,
-		NewSessionID: func() (string, error) { return "child_task", nil },
-		Load: func(LoadOptions) (LoadResult, error) {
-			// No pinned model, so the parent's is the only thing that can supply one.
-			return LoadResult{Specialists: []Manifest{pinnedManifest("", "")}}, nil
-		},
-		RunChild: func(_ context.Context, _ string, args []string, _ func(streamjson.Event)) (ChildRunResult, error) {
-			captured = append([]string(nil), args...)
-			return ChildRunResult{
-				Events: []streamjson.Event{
-					{Type: streamjson.EventRunStart, RunID: "run_1", SessionID: "child_task"},
-					{Type: streamjson.EventFinal, RunID: "run_1", Text: "done"},
-					{Type: streamjson.EventRunEnd, RunID: "run_1", Status: "success", ExitCode: &zero},
-				},
-			}, nil
-		},
-	}
-
-	if _, err := executor.Run(context.Background(), TaskParameters{
-		Name:   "skim",
-		Prompt: "keep going",
-		Resume: "child_task",
-	}, TaskRunOptions{
-		ParentSessionID: parent.SessionID,
-		ParentModel:     "claude-opus-4.1",
-	}); err != nil {
-		t.Fatalf("Run(resume): %v", err)
-	}
-
-	// SETUP: this really was the resume path, not a fresh spawn.
-	if index := indexOf(captured, "--resume"); index < 0 {
-		t.Fatalf("SETUP INVALID: the child was not resumed: %v", captured)
-	}
-	model, ok := argValue(captured, "--model")
-	if !ok {
-		t.Fatalf("the resumed child was launched with no --model, so it runs on whatever the config default resolves to: %v", captured)
-	}
-	if model != "claude-opus-4.1" {
-		t.Fatalf("the resumed child was launched with --model %q, want the parent's %q", model, "claude-opus-4.1")
-	}
-}
-
-// THE FRESH CALL SITE NEEDS THE SAME GUARD, FOR THE SAME REASON.
-//
-// runFresh and runResume both construct their builder input by hand, and both
-// have a byte-identical ParentModel line. Deleting either one compiles. The
-// resume half is what this change repairs; this covers the other half so the
-// pair cannot drift again in the direction nobody was looking.
-func TestRunFreshPassesTheParentModelThroughToTheChild(t *testing.T) {
-	store := sessions.NewStore(sessions.StoreOptions{RootDir: t.TempDir()})
-	parent, err := store.Create(sessions.CreateInput{SessionID: "parent_session"})
-	if err != nil {
-		t.Fatalf("create parent: %v", err)
-	}
-
-	zero := 0
-	var captured []string
-	executor := Executor{
-		BinaryPath:   "/usr/local/bin/zero",
-		SessionStore: store,
-		NewSessionID: func() (string, error) { return "child_task", nil },
-		Load: func(LoadOptions) (LoadResult, error) {
-			return LoadResult{Specialists: []Manifest{pinnedManifest("", "")}}, nil
-		},
-		RunChild: func(_ context.Context, _ string, args []string, _ func(streamjson.Event)) (ChildRunResult, error) {
-			captured = append([]string(nil), args...)
-			return ChildRunResult{
-				Events: []streamjson.Event{
-					{Type: streamjson.EventRunStart, RunID: "run_1", SessionID: "child_task"},
-					{Type: streamjson.EventFinal, RunID: "run_1", Text: "done"},
-					{Type: streamjson.EventRunEnd, RunID: "run_1", Status: "success", ExitCode: &zero},
-				},
-			}, nil
-		},
-	}
-
-	if _, err := executor.Run(context.Background(), TaskParameters{
-		Name:   "skim",
-		Prompt: "find the thing",
-	}, TaskRunOptions{
-		ParentSessionID: parent.SessionID,
-		ParentModel:     "claude-opus-4.1",
-	}); err != nil {
-		t.Fatalf("Run(fresh): %v", err)
-	}
-
-	// SETUP: a fresh spawn, not a resume, or this covers the wrong site.
-	if indexOf(captured, "--resume") >= 0 {
-		t.Fatalf("SETUP INVALID: the child was resumed rather than freshly spawned: %v", captured)
-	}
-	model, ok := argValue(captured, "--model")
-	if !ok || model != "claude-opus-4.1" {
-		t.Fatalf("the fresh child was launched with --model %q (present=%t), want the parent's", model, ok)
-	}
-}

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/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}