fix: tighten command output summary detection

AGENTS.md

@@ -67,39 +67,3 @@ requirements. If a related check fails, fix it. If a required check cannot run
 because of the environment or fails for an unrelated external reason, report
 the exact failure and obtain maintainer direction rather than silently ignoring
 it.
-
-## 4. Common Review Blockers
-
-These classes drive multi-round reviews. Fix them before requesting review:
-
-- **Fresh base:** Rebase onto the current PR base (`main` or stacked target)
-  before review. A stale head that rolls back mainline commits is a hard
-  blocker, not a merge-time detail. Resolve conflicts by keeping upstream
-  behavior unless the PR intentionally changes it.
-- **Platform truth:** Code and tests must pass on Linux, macOS, and Windows.
-  Compare paths after canonicalization (physical/`EvalSymlinks` form); never
-  assert raw `t.TempDir()` spellings (`/var` vs `/private/var` on macOS, short
-  paths on Windows). Prefer one cross-platform function with small `GOOS`
-  checks over duplicated helpers that drift.
-- **Security edges:** Keep secrets out of argv, env dumps, and logs. Redact
-  success and error paths (including stderr). Fail closed on ownership, lease,
-  and permission checks. Do not rely on pre-open path resolution
-  (`EvalSymlinks` then open) for containment: that is a check-to-use race.
-  Bind containment at open/use time with rooted or handle-relative,
-  traversal-resistant APIs. If a no-follow API is used, apply it to every
-  traversed component and enforce the platform's reparse-point protections;
-  final-component-only no-follow is insufficient. On multi-step setup, roll back
-  only what this run created; never destroy pre-existing resources you did
-  not create; never report success when cleanup or unlock failed.
-- **Atomic shared state:** Write a complete temporary file, then atomically
-  replace the destination so concurrent readers never see a partial write.
-  Exclusive create or a write lock alone is not enough for readers. Serialize
-  the full read-modify-write sequence for lockfiles and shared stores.
-- **Tests match the claim:** Every behavior or security-boundary change needs a
-  regression test, including the failure path. Path-sensitive logic needs at
-  least one non-Linux path case (or a hermetic fake that exercises the same
-  normalization).
-- **Honest scope:** PR description, help text, and comments must match what
-  shipped. Wire advertised entry points or shrink the claim. Do not bundle
-  unrelated fixes. Do not widen security allowlists by name alone; check
-  classification and side effects too.

internal/agent/additional_permissions_validation_test.go

@@ -127,7 +127,9 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
 	if !strings.Contains(result.Output, `{"network": {"enabled": true}}`) {
 		t.Fatalf("output = %q, want a concrete additional_permissions example", result.Output)
 	}
-	assertNamesOmitRemedy(t, "missing additional_permissions", result.Output)
+	if !strings.Contains(result.Output, "omit sandbox_permissions") {
+		t.Fatalf("output = %q, want guidance that the flag can simply be omitted", result.Output)
+	}
 }
 
 // A well-formed additional_permissions payload must still reach the normal
@@ -163,123 +165,6 @@ func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T)
 	}
 }
 
-// runShellCall drives one shell tool call through the real registry (and so
-// through Registry.RunWithOptions) exactly as the agent loop would.
-func runShellCall(t *testing.T, root string, arguments string) ToolResult {
-	t.Helper()
-	registry := tools.NewRegistry()
-	registry.Register(tools.NewScopedExecCommandTool(root, nil, nil))
-	result, err := executeToolCall(
-		context.Background(),
-		registry,
-		ToolCall{ID: "c1", Name: "exec_command", Arguments: arguments},
-		PermissionModeUnsafe,
-		Options{Cwd: root},
-	)
-	if err != nil {
-		t.Fatalf("executeToolCall: %v", err)
-	}
-	return result
-}
-
-// TestAdditionalPermissionsRejectionsConverge is the regression guard for
-// #845/#847. A model that attaches a permission object to a command needing
-// none used to be told only "set sandbox_permissions", which pushed it into the
-// OTHER invalid shape (flag set, payload empty) and its "include a permission"
-// error pushed it straight back — the reported loop that burned turns on
-// `git branch --show-current`.
-//
-// The contract this pins is convergence, not tolerance: both shapes are still
-// REJECTED (nothing here grants access), but each rejection must name the one
-// follow-up call that works, and that follow-up must actually succeed.
-func TestAdditionalPermissionsRejectionsConverge(t *testing.T) {
-	root := t.TempDir()
-	const readOnly = `"cmd":"git --version"`
-
-	// Step 1 of the reported loop: a permission object with no flag.
-	first := runShellCall(t, root, `{`+readOnly+`,"additional_permissions":{"network":{"enabled":true}}}`)
-	if first.Status != tools.StatusError {
-		t.Fatalf("unflagged additional_permissions status = %q, want error (rejecting is correct)", first.Status)
-	}
-	assertNamesOmitRemedy(t, "unflagged additional_permissions", first.Output)
-
-	// Step 2 of the reported loop: the model keeps the flag but empties the
-	// payload. This must not be the dead end it was.
-	second := runShellCall(t, root, `{`+readOnly+`,"sandbox_permissions":"with_additional_permissions"}`)
-	if second.Status != tools.StatusError {
-		t.Fatalf("flag without payload status = %q, want error", second.Status)
-	}
-	assertNamesOmitRemedy(t, "flag without payload", second.Output)
-
-	// The call both errors point at must actually work.
-	converged := runShellCall(t, root, `{`+readOnly+`}`)
-	if converged.Status != tools.StatusOK {
-		t.Fatalf("the call both errors instruct the model to make failed: status=%q output=%q", converged.Status, converged.Output)
-	}
-}
-
-// assertNamesOmitRemedy pins the actionable half of the error text: the message
-// must spell out that BOTH fields can simply be dropped. "Set the flag" or
-// "include a permission" alone is what made the two errors chase each other.
-func assertNamesOmitRemedy(t *testing.T, label string, output string) {
-	t.Helper()
-	if !strings.Contains(output, "omit both sandbox_permissions and additional_permissions") {
-		t.Fatalf("%s error = %q, want it to say to omit both fields entirely", label, output)
-	}
-	if !strings.Contains(output, "retry") {
-		t.Fatalf("%s error = %q, want it to tell the model to retry", label, output)
-	}
-}
-
-// The empty-payload rejection carries the same remedy. It is pinned on the
-// validator directly because normalizeNoopAdditionalPermissions absorbs the
-// empty payload before executeToolCall reaches this branch;
-// inlineAdditionalPermissionsProfile is still the shared source of truth that
-// buildPermissionEvent and the grant path re-run, so its text must converge too.
-func TestEmptyAdditionalPermissionsErrorNamesOmitRemedy(t *testing.T) {
-	args := map[string]any{
-		"cmd":                 "git --version",
-		"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
-		"additional_permissions": map[string]any{
-			"network":     map[string]any{"enabled": false},
-			"file_system": map[string]any{"read": []any{}, "write": []any{}},
-		},
-	}
-	_, requested, err := inlineAdditionalPermissionsProfile(args, t.TempDir())
-	if !requested {
-		t.Fatal("requested = false, want true for an explicit with_additional_permissions call")
-	}
-	if err == nil {
-		t.Fatal("an empty additional_permissions payload was accepted; it must still be rejected")
-	}
-	assertNamesOmitRemedy(t, "empty payload", err.Error())
-}
-
-// A provider that materializes every optional property may send
-// "additional_permissions": null. That grants nothing and carries no flag, so it
-// is the field omitted — rejecting it stalled an ordinary command behind an
-// error the model could not act on.
-func TestExecuteToolCallTreatsNullAdditionalPermissionsAsOmitted(t *testing.T) {
-	result := runShellCall(t, t.TempDir(), `{"cmd":"git --version","additional_permissions":null}`)
-	if result.Status != tools.StatusOK {
-		t.Fatalf("null additional_permissions was rejected: status=%q output=%q", result.Status, result.Output)
-	}
-}
-
-// A null payload alongside the flag must still be rejected, and must land on
-// the same error as an absent payload rather than being quietly downgraded.
-func TestExecuteToolCallRejectsFlaggedNullAdditionalPermissions(t *testing.T) {
-	result := runShellCall(t, t.TempDir(),
-		`{"cmd":"git --version","sandbox_permissions":"with_additional_permissions","additional_permissions":null}`)
-	if result.Status != tools.StatusError {
-		t.Fatalf("flagged null status = %q, want error", result.Status)
-	}
-	if !strings.Contains(result.Output, "no additional_permissions object was provided") {
-		t.Fatalf("output = %q, want the missing-payload error", result.Output)
-	}
-	assertNamesOmitRemedy(t, "flagged null", result.Output)
-}
-
 func TestNormalizeProviderExpandedEmptyAdditionalPermissions(t *testing.T) {
 	for _, mode := range []string{
 		string(tools.SandboxPermissionsUseDefault),

internal/agent/loop.go

@@ -656,13 +656,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 		// between tool_results breaks strict provider replay) — same after-batch
 		// rationale as turnRequestedModel above.
 		var changedFilesThisBatch []string
-		// Images produced by tools this turn, held until every tool_result is
-		// recorded. They travel as user messages, and a user message between two
-		// tool_results breaks strict provider replay — Anthropic coalesces them
-		// into one user block list and requires the tool_result blocks first, so
-		// interleaving yields [tool_result, text, image, tool_result] and a 400.
-		// Same reason the self-correction feedback below is deferred.
-		var toolImageMessages []zeroruntime.Message
 		// Parallel read-ahead state: results for calls[precomputedStart:precomputedEnd]
 		// executed concurrently, consumed strictly in order below.
 		var precomputed []precomputedToolResult
@@ -716,16 +709,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				Content:    toolResult.Output,
 				ToolCallID: toolResult.ToolCallID,
 			})
-			// Images ride a following USER message rather than the tool result
-			// above. Every provider drops images on a tool-role message —
-			// Anthropic's tool_result content is a string, Gemini's is a
-			// functionResponse, and OpenAI guards its image parts to the user role
-			// — so attaching them there would silently deliver nothing. A separate
-			// message also keeps the one-tool-result-per-tool-call pairing intact,
-			// which the providers validate.
-			if imageMessage, ok := toolResultImageMessage(toolResult); ok {
-				toolImageMessages = append(toolImageMessages, imageMessage)
-			}
 
 			// A tool may demand the run ABORT — a canceled/timed-out ask_user prompt
 			// returns context.Canceled rather than fabricating a headless answer. Stop
@@ -736,13 +719,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 			}
 			if abortErr != nil {
 				messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
-				messages = append(messages, toolImageMessages...)
 				result.Messages = copyMessages(messages)
 				return result, abortErr
 			}
 			if stopReason := stopReasonFromToolResult(toolResult); stopReason != "" {
 				messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
-				messages = append(messages, toolImageMessages...)
 				result.FinalAnswer = toolResult.Output
 				result.StopReason = stopReason
 				result.Messages = copyMessages(messages)
@@ -766,7 +747,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				// messages stay valid for a strict provider replay (Anthropic
 				// rejects a tool_use with no answering tool_result).
 				messages = appendAbortedToolResults(messages, collected.ToolCalls[index+1:])
-				messages = append(messages, toolImageMessages...)
 				result.FinalAnswer = toolFailureStopAnswer(call.Name, outcome.Count)
 				result.Messages = copyMessages(messages)
 				return result, nil
@@ -785,10 +765,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				postEditDiagnostics.enqueue(ctx, toolResult.ChangedFiles)
 			}
 		}
-		// Every tool_result for this turn is now recorded, including aborted
-		// placeholders, so the images can follow without splitting them.
-		messages = append(messages, toolImageMessages...)
-		toolImageMessages = nil
 
 		// Run post-edit self-correction once over the union of files this turn
 		// changed, then append any feedback after every tool_result is recorded so
@@ -1460,7 +1436,6 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 		Output:          result.Output,
 		Truncated:       result.Truncated,
 		Meta:            result.Meta,
-		Images:          result.Images,
 		Redacted:        result.Redacted,
 		ChangedFiles:    result.ChangedFiles,
 		ChangeSummaries: result.ChangeSummaries,
@@ -2788,16 +2763,7 @@ func shellCommandAdditionalPermissionsRequested(args map[string]any) bool {
 // access and malformed payloads are left untouched so validation stays closed.
 func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, engine *sandbox.Engine) {
 	raw, exists := args["additional_permissions"]
-	if !exists {
-		return
-	}
-	// An explicit null is the field omitted, not a request: drop the key so the
-	// rest of the pipeline sees the same args a model that omitted it would send.
-	// sandbox_permissions is deliberately left alone — a flagged null must reach
-	// the same "no additional_permissions object was provided" error as a flagged
-	// absent field rather than being silently downgraded to a different outcome.
-	if raw == nil {
-		delete(args, "additional_permissions")
+	if !exists || raw == nil {
 		return
 	}
 	data, err := json.Marshal(raw)
@@ -2828,41 +2794,21 @@ func normalizeNoopAdditionalPermissions(args map[string]any, basePath string, en
 	}
 }
 
-// omitAdditionalPermissionsHint is the one instruction that ends the retry loop
-// in #845/#847. Every additional_permissions rejection must carry it: the model
-// reaches these errors by attaching a permission object to a command that needs
-// none, so "omit it and retry" is the answer in practice, and an error that
-// only explains how to build a VALID permission object sends the model round
-// the loop again with a different invalid shape.
-const omitAdditionalPermissionsHint = "If this command does not need extra sandbox access — ordinary " +
-	"read-only commands such as `git branch --show-current` do not — omit both sandbox_permissions and " +
-	"additional_permissions entirely and retry; that is the fix in almost every case."
-
 func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (sandbox.RequestPermissionProfile, bool, error) {
 	if !shellCommandAdditionalPermissionsRequested(args) {
-		// An explicit null grants nothing and carries no flag, so it is
-		// indistinguishable from an omitted field; rejecting it would be a pure
-		// false positive against providers that materialize every optional
-		// property.
-		if raw, exists := args["additional_permissions"]; exists && raw != nil {
-			return sandbox.RequestPermissionProfile{}, false, fmt.Errorf(
-				"additional_permissions was provided without sandbox_permissions set to %q, so it cannot be applied. %s "+
-					"Only if the command genuinely needs file or network access the sandbox denies, resend with "+
-					`sandbox_permissions: %q AND a non-empty additional_permissions, for example {"network": {"enabled": true}}`,
-				tools.SandboxPermissionsWithAdditionalPermissions,
-				omitAdditionalPermissionsHint,
-				tools.SandboxPermissionsWithAdditionalPermissions)
+		if _, exists := args["additional_permissions"]; exists {
+			return sandbox.RequestPermissionProfile{}, false, fmt.Errorf("additional_permissions requires sandbox_permissions set to %q", tools.SandboxPermissionsWithAdditionalPermissions)
 		}
 		return sandbox.RequestPermissionProfile{}, false, nil
 	}
 	raw, ok := args["additional_permissions"]
 	if !ok || raw == nil {
 		return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
-			"sandbox_permissions was set to %q but no additional_permissions object was provided. %s "+
-				`Otherwise include one, for example additional_permissions: {"network": {"enabled": true}} or `+
-				`{"file_system": {"write": ["/path"]}}`,
-			tools.SandboxPermissionsWithAdditionalPermissions,
-			omitAdditionalPermissionsHint)
+			"sandbox_permissions was set to %q but no additional_permissions object was provided. "+
+				`Include one, for example additional_permissions: {"network": {"enabled": true}} or `+
+				`{"file_system": {"write": ["/path"]}}. If this command does not need elevated permissions, `+
+				"omit sandbox_permissions entirely and retry",
+			tools.SandboxPermissionsWithAdditionalPermissions)
 	}
 	data, err := json.Marshal(raw)
 	if err != nil {
@@ -2878,10 +2824,8 @@ func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (s
 	}
 	if normalized.Empty() {
 		return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
-			"additional_permissions granted nothing, so it cannot be applied. %s "+
-				`Otherwise include a real permission, for example {"network": {"enabled": true}} or `+
-				`{"file_system": {"write": ["/path"]}}`,
-			omitAdditionalPermissionsHint)
+			"additional_permissions must include at least one of network or file_system, for example " +
+				`{"network": {"enabled": true}} or {"file_system": {"write": ["/path"]}}`)
 	}
 	grantProfile, err := sandbox.RequestPermissionGrantProfile(normalized)
 	if err != nil {
@@ -3314,37 +3258,3 @@ func copyMessages(messages []Message) []Message {
 	}
 	return copied
 }
-
-// toolResultImageMessage builds the user message that carries a tool's images
-// to the model, or reports false when the tool produced none.
-//
-// The text names the tool so the model can tell which call an image came from
-// when several ran in one turn — the images arrive detached from their tool
-// result, so nothing else associates them.
-func toolResultImageMessage(result ToolResult) (zeroruntime.Message, bool) {
-	images := make([]zeroruntime.ImageBlock, 0, len(result.Images))
-	for _, image := range result.Images {
-		if len(image.Data) == 0 {
-			continue
-		}
-		mediaType := zeroruntime.NormalizeImageMediaType(image.MediaType)
-		if mediaType == "" {
-			// Outside the provider allow-list. Dropping it beats sending bytes a
-			// provider will reject and failing the whole turn.
-			continue
-		}
-		images = append(images, zeroruntime.ImageBlock{MediaType: mediaType, Data: image.Data})
-	}
-	if len(images) == 0 {
-		return zeroruntime.Message{}, false
-	}
-	label := result.Name
-	if label == "" {
-		label = "tool"
-	}
-	return zeroruntime.Message{
-		Role:    zeroruntime.MessageRoleUser,
-		Content: "Image output from " + label + ":",
-		Images:  images,
-	}, true
-}

internal/agent/system_prompt.md

@@ -30,13 +30,11 @@ work.
    read-before-edit discipline: inspect the target file and nearby callers,
    tests, or config before you modify behavior. Never edit a file you have not
    read.
-2. **Plan.** Use update_plan for implementation or investigation that has at
-   least three meaningful dependent steps, then keep it current at milestones.
-   Mark completed work and the next in-progress step together when practical;
-   do not spend separate turns updating it after every file or command. Keep at
-   most one item in_progress. Skip plans for simple lookups, explanations, code
-   navigation, and other short tasks. Never create a plan after the work is
-   already complete merely to describe what you did.
+2. **Plan.** For multi-step work, call update_plan with an ordered checklist and
+   keep it current at meaningful milestones. Mark completed work and the next
+   in-progress step together when practical; do not spend separate model turns
+   updating the plan after every file or command. Keep at most one item
+   in_progress. Skip the plan for trivial one-step tasks.
 3. **Implement.** Make focused changes that match the surrounding code's style,
    naming, and conventions. Prefer the smallest change that fully solves the
    problem. Avoid broad refactors, unrelated rewrites, dependency churn, and
@@ -51,13 +49,9 @@ work.
 ## Editing discipline
 
 - Choose the narrowest tool that safely accomplishes the step. Prefer native
-  file tools - read_file, read_minified_file, list_directory, glob, grep,
-  write_file, edit_file, apply_patch - over shelling out to
-  cat/sed/awk/python for file operations.
+  file tools - read_file, list_directory, glob, grep, write_file, edit_file,
+  apply_patch - over shelling out to cat/sed/awk/python for file operations.
   They are safer, reviewable, and produce clean diffs.
-- Prefer read_minified_file when initially exploring source code; it preserves
-  code while removing comments and redundant whitespace. Use read_file when
-  exact text, comments, or line numbers are needed.
 - Keep edits focused and reviewable. A single patch may update several related
   files when they form one coherent change; do not hide unrelated edits in a
   bulk shell or script rewrite.

internal/agent/system_prompt_models.go

@@ -71,6 +71,5 @@ const geminiPromptAddendum = `<model_guidance>
   equivalent shell commands; they are safer and produce cleaner diffs.
 - Be concise and concrete. When you run a shell command with side effects, state
   in one short clause why it is needed.
-- Use update_plan only for work with at least three meaningful dependent steps;
-  skip it for simple lookups and never create one after the work is complete.
+- Use update_plan for any multi-step task and keep it current.
 </model_guidance>`

internal/agent/tool_result_images_test.go

@@ -1,207 +0,0 @@
-package agent
-
-import (
-	"context"
-	"fmt"
-	"strings"
-	"testing"
-
-	"github.com/Gitlawb/zero/internal/tools"
-	"github.com/Gitlawb/zero/internal/zeroruntime"
-)
-
-// imageTool returns a tool result carrying an image, the way a screenshot tool
-// should be able to.
-type imageTool struct {
-	media string
-	data  []byte
-}
-
-func (imageTool) Name() string        { return "capture" }
-func (imageTool) Description() string { return "Captures an image." }
-func (imageTool) Parameters() tools.Schema {
-	return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
-}
-func (imageTool) Safety() tools.Safety {
-	return tools.Safety{Permission: tools.PermissionAllow}
-}
-func (t imageTool) Run(context.Context, map[string]any) tools.Result {
-	return tools.Result{
-		Status: tools.StatusOK,
-		Output: "Captured a screenshot.",
-		Images: []zeroruntime.ImageBlock{{MediaType: t.media, Data: t.data}},
-	}
-}
-
-// A tool that produces an image must get that image in front of the model.
-//
-// It cannot ride the tool-result message: every provider drops images there.
-// Anthropic maps a tool result to a tool_result block with string content,
-// Gemini to a functionResponse, and OpenAI guards its image parts to the user
-// role explicitly. So the image has to arrive as a following user message,
-// which is also the only shape that keeps one tool result per tool call.
-func TestRunDeliversToolResultImagesToTheModel(t *testing.T) {
-	registry := tools.NewRegistry()
-	registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
-
-	provider := &mockProvider{
-		turns: [][]zeroruntime.StreamEvent{
-			{
-				{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call_1", ToolName: "capture"},
-				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call_1", ArgumentsFragment: `{}`},
-				{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call_1"},
-				{Type: zeroruntime.StreamEventDone},
-			},
-			{
-				{Type: zeroruntime.StreamEventText, Content: "I can see it."},
-				{Type: zeroruntime.StreamEventDone},
-			},
-		},
-	}
-
-	result, err := Run(context.Background(), "screenshot please", provider, Options{
-		Registry: registry,
-		MaxTurns: 2,
-	})
-	if err != nil {
-		t.Fatalf("Run: %v", err)
-	}
-
-	var carrier *zeroruntime.Message
-	for index := range result.Messages {
-		if len(result.Messages[index].Images) > 0 {
-			carrier = &result.Messages[index]
-			break
-		}
-	}
-	if carrier == nil {
-		t.Fatal("no message carried the tool's image; the model never saw it")
-	}
-	if carrier.Role != zeroruntime.MessageRoleUser {
-		t.Errorf("image rode a %q message; every provider only serializes images on the user role", carrier.Role)
-	}
-	if got := carrier.Images[0].MediaType; got != "image/png" {
-		t.Errorf("media type = %q, want image/png", got)
-	}
-
-	// The tool-result pairing must be untouched: one tool result per tool call.
-	toolResults := 0
-	for _, message := range result.Messages {
-		if message.Role == zeroruntime.MessageRoleTool {
-			toolResults++
-		}
-	}
-	if toolResults != 1 {
-		t.Errorf("got %d tool-result messages for 1 tool call", toolResults)
-	}
-}
-
-// textTool is a second, image-free tool so a turn can carry two calls.
-type textTool struct{}
-
-func (textTool) Name() string        { return "note" }
-func (textTool) Description() string { return "Notes something." }
-func (textTool) Parameters() tools.Schema {
-	return tools.Schema{Type: "object", Properties: map[string]tools.PropertySchema{}}
-}
-func (textTool) Safety() tools.Safety { return tools.Safety{Permission: tools.PermissionAllow} }
-func (textTool) Run(context.Context, map[string]any) tools.Result {
-	return tools.Result{Status: tools.StatusOK, Output: "noted"}
-}
-
-// Tool results for one assistant turn must stay contiguous. The image messages
-// are user-role, and a user message between two tool_results breaks strict
-// provider replay: Anthropic coalesces consecutive user content into one block
-// list and requires tool_result blocks first, so interleaving produces
-// [tool_result, text, image, tool_result] and the request is rejected.
-//
-// The first version of this feature appended each image immediately after its
-// own tool result, which is exactly that shape. A single-tool-call test passed
-// and said nothing about it.
-func TestRunKeepsToolResultsContiguousWhenAToolReturnsAnImage(t *testing.T) {
-	registry := tools.NewRegistry()
-	registry.Register(imageTool{media: "image/png", data: []byte("\x89PNG\r\n\x1a\nfake")})
-	registry.Register(textTool{})
-
-	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
-		{
-			{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c1", ToolName: "capture"},
-			{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c1", ArgumentsFragment: `{}`},
-			{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c1"},
-			{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "c2", ToolName: "note"},
-			{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "c2", ArgumentsFragment: `{}`},
-			{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "c2"},
-			{Type: zeroruntime.StreamEventDone},
-		},
-		{{Type: zeroruntime.StreamEventText, Content: "done"}, {Type: zeroruntime.StreamEventDone}},
-	}}
-
-	result, err := Run(context.Background(), "two calls", provider, Options{Registry: registry, MaxTurns: 2})
-	if err != nil {
-		t.Fatalf("Run: %v", err)
-	}
-
-	// Two separate ifs rather than a switch: a tool-role message that also
-	// carried images would match only the first arm of a switch, so a regression
-	// that put the image back on the tool result would read as "no image at all"
-	// instead of naming what actually happened.
-	type toolResult struct {
-		index int
-		id    string
-	}
-	var toolResults []toolResult
-	imageIndex := -1
-	for index, message := range result.Messages {
-		if message.Role == zeroruntime.MessageRoleTool {
-			toolResults = append(toolResults, toolResult{index: index, id: message.ToolCallID})
-		}
-		if len(message.Images) > 0 {
-			if imageIndex >= 0 {
-				t.Fatalf("image delivered twice, at %d and %d; recorded %s", imageIndex, index, messageShape(result.Messages))
-			}
-			imageIndex = index
-		}
-	}
-	if imageIndex < 0 {
-		t.Fatalf("the image never reached the model; recorded %s", messageShape(result.Messages))
-	}
-	if len(toolResults) != 2 {
-		t.Fatalf("got %d tool-result messages for 2 tool calls; recorded %s", len(toolResults), messageShape(result.Messages))
-	}
-	if toolResults[0].id != "c1" || toolResults[1].id != "c2" {
-		t.Errorf("tool results answered %q then %q, want c1 then c2", toolResults[0].id, toolResults[1].id)
-	}
-	// The contract Anthropic enforces: every tool_result block has to come before
-	// any other block in the user message it lands in. Anything wedged between
-	// the two results breaks that, which is the 400 this PR exists to avoid.
-	if toolResults[1].index != toolResults[0].index+1 {
-		t.Errorf("tool results at %d and %d are not adjacent; %q message at %d splits them; recorded %s",
-			toolResults[0].index, toolResults[1].index, result.Messages[toolResults[0].index+1].Role,
-			toolResults[0].index+1, messageShape(result.Messages))
-	}
-	// Follows the last tool result, deliberately NOT pinned to exactly last+1.
-	// appendUserBlocks coalesces consecutive user-role content, so
-	// [tool_result, tool_result, text, image] still maps to tool_result blocks
-	// first and replays fine; pinning would redden that valid shape for nothing.
-	if imageIndex <= toolResults[1].index {
-		t.Errorf("image message at %d does not follow the last tool result at %d; recorded %s",
-			imageIndex, toolResults[1].index, messageShape(result.Messages))
-	}
-}
-
-// messageShape renders the conversation as a compact role list so a failure says
-// what the ordering actually was instead of dumping whole messages.
-func messageShape(messages []zeroruntime.Message) string {
-	parts := make([]string, 0, len(messages))
-	for index, message := range messages {
-		part := fmt.Sprintf("%d:%s", index, message.Role)
-		if message.ToolCallID != "" {
-			part += "(" + message.ToolCallID + ")"
-		}
-		if len(message.Images) > 0 {
-			part += "+image"
-		}
-		parts = append(parts, part)
-	}
-	return "[" + strings.Join(parts, " ") + "]"
-}

internal/agent/types.go

@@ -68,11 +68,8 @@ type ToolResult struct {
 	Output     string
 	// Truncated reports that the tool's model-visible output omitted content.
 	// The full result may be recoverable through Meta["spill_path"].
-	Truncated bool
-	Meta      map[string]string
-	// Images the tool produced, delivered to the model as a following user
-	// message rather than on this result. See tools.Result.Images.
-	Images       []zeroruntime.ImageBlock
+	Truncated    bool
+	Meta         map[string]string
 	Redacted     bool
 	ChangedFiles []string
 	// ChangeSummaries are non-selectable generated-tree summaries emitted by

internal/cli/deferred_wiring_test.go

@@ -79,8 +79,8 @@ func TestRegisterToolSearchIfEligibleSkipsWhenThresholdZero(t *testing.T) {
 
 func TestDeferredEligibleCountIncludesOptionalBuiltins(t *testing.T) {
 	registry := newCoreRegistry(t.TempDir())
-	if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got < 2 {
-		t.Fatalf("deferredEligibleCount(core) = %d, want at least 2 optional built-ins", got)
+	if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got < 3 {
+		t.Fatalf("deferredEligibleCount(core) = %d, want at least 3 optional built-ins", got)
 	}
 }
 

internal/cli/mcp_config.go

@@ -308,34 +308,6 @@ func runMCPCheck(ctx context.Context, args []string, stdout io.Writer, stderr io
 	}
 	defer closeMCPRuntime(stderr, mcpRuntime)
 
-	// A connect failure does not come back as err. RegisterTools is deliberately
-	// best-effort so one unreachable server cannot stop Zero launching: it records
-	// the failure on the runtime and returns nil. That is right for startup and
-	// wrong to ignore here, because this is the command a user runs precisely when
-	// a server is misbehaving, and reporting "is reachable" for a server that never
-	// started sends them looking somewhere else.
-	for _, skipped := range mcpRuntime.Skipped() {
-		if skipped.Name != serverName {
-			continue
-		}
-		message := fmt.Sprintf("MCP server %s is not reachable", serverName)
-		if skipped.Err != nil {
-			message = fmt.Sprintf("%s: %s", message, redaction.ErrorMessage(skipped.Err, redaction.Options{}))
-		}
-		if options.json {
-			payload := struct {
-				ServerName string `json:"serverName"`
-				Status     string `json:"status"`
-				Error      string `json:"error,omitempty"`
-			}{ServerName: serverName, Status: "unreachable", Error: message}
-			if err := writePrettyJSON(stdout, redaction.RedactValue(payload, redaction.Options{})); err != nil {
-				return exitCrash
-			}
-			return exitCrash
-		}
-		return writeAppError(stderr, message, exitCrash)
-	}
-
 	items := mcpToolList(registry)
 	if options.json {
 		payload := struct {

internal/cli/mcp_trust_edge_test.go

@@ -6,7 +6,6 @@ package cli
 
 import (
 	"bytes"
-	"encoding/json"
 	"errors"
 	"os"
 	"path/filepath"
@@ -220,81 +219,3 @@ func TestRunMCPCheckNormalizeError(t *testing.T) {
 		t.Fatalf("want normalize error, got %q", got)
 	}
 }
-
-// TestRunMCPCheckReportsUnreachableServer covers the case the command exists
-// for: a server that fails to start.
-//
-// RegisterTools is best-effort by design, so a connect failure is recorded on
-// the runtime and registration returns nil. runMCPCheck used to inspect only
-// that nil error and then print "is reachable", which told the user their
-// broken server was fine. The command is spawned for real here rather than
-// stubbed, because the whole failure mode was a mismatch between what
-// registration returns and what it records.
-func TestRunMCPCheckReportsUnreachableServer(t *testing.T) {
-	setTrustConfigRoot(t)
-	deps := appDeps{
-		getwd: func() (string, error) { return t.TempDir(), nil },
-		resolveMCPConfig: func(_ string, _ bool) (config.MCPConfig, error) {
-			return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
-				"broken": {Type: "stdio", Command: "zero-definitely-not-a-real-binary-xyz"},
-			}}, nil
-		},
-	}
-	var out, errBuf bytes.Buffer
-	if code := runWithDeps([]string{"mcp", "check", "broken"}, &out, &errBuf, deps); code == exitSuccess {
-		t.Fatalf("mcp check must fail for a server that did not start; stdout=%q stderr=%q", out.String(), errBuf.String())
-	}
-	if got := out.String(); strings.Contains(got, "is reachable") {
-		t.Fatalf("mcp check reported a failed server as reachable: %q", got)
-	}
-	if got := errBuf.String(); !strings.Contains(got, "not reachable") {
-		t.Fatalf("want an unreachable error naming the server, got %q", got)
-	}
-}
-
-// The --json contract for an unreachable server, which the text case above does
-// not touch.
-//
-// The JSON branch is a separate return path with its own payload, so a caller
-// parsing it would otherwise be relying on a shape nothing pins: reverting the
-// check leaves it emitting status "ok" with a tool count, which is valid JSON
-// and completely wrong.
-func TestRunMCPCheckReportsUnreachableServerJSON(t *testing.T) {
-	setTrustConfigRoot(t)
-	deps := appDeps{
-		getwd: func() (string, error) { return t.TempDir(), nil },
-		resolveMCPConfig: func(_ string, _ bool) (config.MCPConfig, error) {
-			return config.MCPConfig{Servers: map[string]config.MCPServerConfig{
-				"broken": {Type: "stdio", Command: "zero-definitely-not-a-real-binary-xyz"},
-			}}, nil
-		},
-	}
-	var out, errBuf bytes.Buffer
-	if code := runWithDeps([]string{"mcp", "check", "broken", "--json"}, &out, &errBuf, deps); code == exitSuccess {
-		t.Fatalf("mcp check --json must fail for a server that did not start; stdout=%q stderr=%q", out.String(), errBuf.String())
-	}
-	var payload struct {
-		ServerName string `json:"serverName"`
-		Status     string `json:"status"`
-		Error      string `json:"error"`
-		ToolCount  int    `json:"toolCount"`
-	}
-	if err := json.Unmarshal(out.Bytes(), &payload); err != nil {
-		t.Fatalf("decode mcp check --json output %q: %v", out.String(), err)
-	}
-	if payload.ServerName != "broken" {
-		t.Fatalf("serverName = %q, want the server that was checked", payload.ServerName)
-	}
-	if payload.Status != "unreachable" {
-		t.Fatalf("status = %q, want %q", payload.Status, "unreachable")
-	}
-	if !strings.Contains(payload.Error, "not reachable") {
-		t.Fatalf("error = %q, want it to say the server is not reachable", payload.Error)
-	}
-	// A reachable payload carries a tool count; an unreachable one must not
-	// imply zero tools were found, which reads as a server that simply exposes
-	// none.
-	if payload.ToolCount != 0 {
-		t.Fatalf("toolCount = %d, want it absent from an unreachable payload", payload.ToolCount)
-	}
-}

internal/minify/minify.go

@@ -13,10 +13,8 @@ package minify
 
 import (
 	"bytes"
-	"go/ast"
 	"go/parser"
 	"go/printer"
-	"go/scanner"
 	"go/token"
 	"path/filepath"
 	"strings"
@@ -36,11 +34,8 @@ func File(path string, content []byte) Result {
 		if out, ok := minifyGo(content); ok {
 			return Result{Content: out, Language: "go", Applied: true}
 		}
-		if out, ok := minifyGoFragment(content); ok {
-			return Result{Content: out, Language: "go-fragment", Applied: true}
-		}
-		// An incomplete fragment that cannot be wrapped safely falls through to
-		// whitespace-only normalization; no source text is guessed or discarded.
+		// Unparsable Go (a snippet, generics edge, or syntax error): fall through
+		// to the safe generic path rather than risk a partial AST reprint.
 	} else if style, ok := commentStyles[ext]; ok {
 		// Strip comments with a string-aware lexer, then collapse the whitespace the
 		// removed comments left behind. The stripper only handles languages whose
@@ -51,133 +46,6 @@ func File(path string, content []byte) Result {
 	return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
 }
 
-// Fragment minifies a bounded source range without assuming lexical state from
-// text outside the range. Go remains parser-validated; other languages use the
-// conservative whitespace-only transform because a range may begin inside a
-// multiline string or block comment.
-func Fragment(path string, content []byte) Result {
-	if strings.EqualFold(filepath.Ext(path), ".go") {
-		if out, ok := minifyGo(content); ok {
-			return Result{Content: out, Language: "go", Applied: true}
-		}
-		if out, ok := minifyGoFragment(content); ok {
-			return Result{Content: out, Language: "go-fragment", Applied: true}
-		}
-	}
-	return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
-}
-
-// ContextualFragment minifies a bounded fragment after checking its starting
-// position against the complete source. A Go range that begins inside a string
-// or comment must remain verbatim apart from whitespace normalization: parsing
-// that fragment by itself would mistake literal text for Go syntax.
-func ContextualFragment(path string, source, content []byte, startOffset int) Result {
-	if strings.EqualFold(filepath.Ext(path), ".go") && goFragmentStartsInsideToken(source, startOffset) {
-		return Result{Content: minifyGeneric(content), Language: "text", Applied: false}
-	}
-	return Fragment(path, content)
-}
-
-func goFragmentStartsInsideToken(source []byte, startOffset int) bool {
-	if startOffset <= 0 || startOffset >= len(source) {
-		return false
-	}
-	fset := token.NewFileSet()
-	file := fset.AddFile("", fset.Base(), len(source))
-	var lexer scanner.Scanner
-	lexer.Init(file, source, nil, scanner.ScanComments)
-	for {
-		position, kind, literal := lexer.Scan()
-		if kind == token.EOF {
-			return false
-		}
-		if kind != token.STRING && kind != token.COMMENT {
-			continue
-		}
-		start := file.Offset(position)
-		if startOffset > start && startOffset < start+len(literal) {
-			return true
-		}
-	}
-}
-
-// minifyGoFragment handles bounded reads that do not contain a package clause.
-// It finds the longest complete declaration or statement prefix that the real
-// Go parser accepts, prints that prefix without comments, and preserves any
-// incomplete tail conservatively. Ranges are intentionally capped: repeatedly
-// parsing a large broken file would otherwise turn a cheap read into O(n²) work.
-func minifyGoFragment(content []byte) (string, bool) {
-	lines := strings.Split(strings.ReplaceAll(string(content), "\r\n", "\n"), "\n")
-	if len(lines) > 1 && lines[len(lines)-1] == "" {
-		lines = lines[:len(lines)-1]
-	}
-	if len(lines) > 512 {
-		return "", false
-	}
-	for end := len(lines); end > 0; end-- {
-		prefix := strings.Join(lines[:end], "\n")
-		if compact, ok := parseGoDeclarations(prefix); ok {
-			return joinCompactPrefix(compact, lines[end:]), true
-		}
-		if compact, ok := parseGoStatements(prefix); ok {
-			return joinCompactPrefix(compact, lines[end:]), true
-		}
-	}
-	return "", false
-}
-
-func parseGoDeclarations(fragment string) (string, bool) {
-	fset := token.NewFileSet()
-	file, err := parser.ParseFile(fset, "", "package compact\n"+fragment, parser.SkipObjectResolution)
-	if err != nil || len(file.Decls) == 0 {
-		return "", false
-	}
-	nodes := make([]ast.Node, len(file.Decls))
-	for i, declaration := range file.Decls {
-		nodes[i] = declaration
-	}
-	return printGoNodes(fset, nodes)
-}
-
-func parseGoStatements(fragment string) (string, bool) {
-	fset := token.NewFileSet()
-	file, err := parser.ParseFile(fset, "", "package compact\nfunc _(){\n"+fragment+"\n}", parser.SkipObjectResolution)
-	if err != nil || len(file.Decls) != 1 {
-		return "", false
-	}
-	function, ok := file.Decls[0].(*ast.FuncDecl)
-	if !ok || function.Body == nil || len(function.Body.List) == 0 {
-		return "", false
-	}
-	nodes := make([]ast.Node, len(function.Body.List))
-	for i, statement := range function.Body.List {
-		nodes[i] = statement
-	}
-	return printGoNodes(fset, nodes)
-}
-
-func printGoNodes(fset *token.FileSet, nodes []ast.Node) (string, bool) {
-	var buf bytes.Buffer
-	cfg := printer.Config{Mode: printer.TabIndent, Tabwidth: 1}
-	for i, node := range nodes {
-		if i > 0 {
-			buf.WriteByte('\n')
-		}
-		if err := cfg.Fprint(&buf, fset, node); err != nil {
-			return "", false
-		}
-	}
-	return strings.TrimSpace(buf.String()), true
-}
-
-func joinCompactPrefix(compact string, remainder []string) string {
-	tail := minifyGeneric([]byte(strings.Join(remainder, "\n")))
-	if tail == "" {
-		return compact
-	}
-	return compact + "\n" + tail
-}
-
 // minifyGo parses Go WITHOUT comments (omitting parser.ParseComments leaves them
 // unattached) and reprints the AST, yielding valid, comment-free, gofmt-shaped
 // source with tab indentation (1 char per level). It returns ok=false on any

internal/minify/minify_test.go

@@ -39,58 +39,17 @@ func Greet(name string) string {
 	}
 }
 
-func TestMinifyGoStatementFragment(t *testing.T) {
+func TestMinifyGoFallsBackOnUnparsableGo(t *testing.T) {
+	// A snippet (no package clause) cannot parse as a file -> safe generic path.
 	r := File("snippet.go", []byte("x := 1   \n\n\n// keep me\ny := 2\n"))
-	if !r.Applied || r.Language != "go-fragment" {
-		t.Fatalf("expected parsed Go fragment, got %+v", r)
+	if r.Applied {
+		t.Fatalf("expected fallback for unparsable Go, got %+v", r)
 	}
 	if strings.Contains(r.Content, "\n\n\n") {
-		t.Errorf("fragment should collapse blank runs:\n%q", r.Content)
+		t.Errorf("generic should collapse blank runs:\n%q", r.Content)
 	}
-	if strings.Contains(r.Content, "keep me") || !strings.Contains(r.Content, "y := 2") {
-		t.Errorf("fragment was not compacted safely: %q", r.Content)
-	}
-}
-
-func TestMinifyGoFragmentAccepts512LinesWithFinalNewline(t *testing.T) {
-	src := strings.Repeat("x++\n", 512)
-	r := File("snippet.go", []byte(src))
-	if !r.Applied || r.Language != "go-fragment" {
-		t.Fatalf("expected 512-line fragment to remain eligible, got %+v", r)
-	}
-	if got := strings.Count(r.Content, "x++"); got != 512 {
-		t.Fatalf("compacted statement count = %d, want 512", got)
-	}
-}
-
-func TestMinifyGoDeclarationPrefixPreservesIncompleteTail(t *testing.T) {
-	src := `func Handle737(n int) (int, error) {
-	if n%2 == 1 {
-		return 0, errBad
-	}
-	return n + 737, nil
-}
-
-// Handle738 begins outside the requested range.
-func Handle738(n int) (int, error) {
-	if n%2 == 1 {`
-	r := File("range.go", []byte(src))
-	if !r.Applied || r.Language != "go-fragment" {
-		t.Fatalf("expected declaration-prefix compaction, got %+v", r)
-	}
-	if strings.Contains(r.Content, "Handle737 returns") || !strings.Contains(r.Content, "return n + 737") {
-		t.Fatalf("complete declaration was not compacted correctly:\n%s", r.Content)
-	}
-	if !strings.Contains(r.Content, "func Handle738") || !strings.Contains(r.Content, "if n%2 == 1 {") {
-		t.Fatalf("incomplete tail was not preserved:\n%s", r.Content)
-	}
-}
-
-func TestMinifyGoLargeInvalidInputFallsBackConservatively(t *testing.T) {
-	src := strings.Repeat("x := 1 // keep\n", 513)
-	r := File("broken.go", []byte(src))
-	if r.Applied || !strings.Contains(r.Content, "// keep") {
-		t.Fatalf("large invalid input must retain conservative fallback: %+v", r)
+	if !strings.Contains(r.Content, "// keep me") {
+		t.Errorf("generic must NOT strip comments (unsafe without a parser): %q", r.Content)
 	}
 }
 

internal/sandbox/runner_windows_integration_test.go

@@ -9,7 +9,6 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -387,127 +386,6 @@ func runWindowsRealSmokeCommand(t *testing.T, runnerExe string, base WindowsSand
 	}
 }
 
-// The write jail must hold on a path whose DACL grants Everyone write access.
-//
-// A WRITE_RESTRICTED token runs TWO checks for a write and needs both to pass:
-// the normal one against its enabled SIDs, and a second against its RESTRICTED
-// SID list. The jail is built on the second check only succeeding where Zero has
-// explicitly ACL'd a capability SID. Putting the World SID (S-1-1-0) in the
-// restricted list breaks that globally — every principal is a member of
-// Everyone, so on a DACL that grants Everyone write, the restricted half passes
-// for free and confinement collapses back to the ordinary user's own
-// permissions, which is precisely the boundary the sandbox exists to be
-// stricter than.
-//
-// The runner already states this rule for the SIDs it refuses to add ("None of
-// the granted SIDs can be added to the restricted list without collapsing the
-// write jail"); Everyone was in the list anyway, since the original sandbox
-// baseline.
-//
-// Realistic rather than theoretical: administrators open share roots with
-// Everyone:F, and third-party installers ship loose ACLs. It needs no privilege,
-// no symlink and no race. Checked and ruled out: C:\Users\Public\Documents
-// grants BATCH, not Everyone, so the common "public folder" case is not this.
-func TestWindowsRestrictedTokenDeniesWritesToEveryoneWritablePaths(t *testing.T) {
-	if os.Getenv("ZERO_SANDBOX_REAL_SMOKE") != "1" {
-		t.Skip("set ZERO_SANDBOX_REAL_SMOKE=1 to run real Windows sandbox smoke tests")
-	}
-	runnerExe := realSmokeExecutable(t, "ZERO_WINDOWS_COMMAND_RUNNER_EXE", WindowsSandboxCommandRunnerName)
-
-	root := t.TempDir()
-	outside := t.TempDir()
-	everyoneDir := filepath.Join(outside, "everyone-writable")
-	if err := os.MkdirAll(everyoneDir, 0o700); err != nil {
-		t.Fatalf("MkdirAll everyone-writable: %v", err)
-	}
-	// Granted through the production ACL applier, so the hostile DACL is built
-	// the same way a real one is rather than by a test-only shortcut.
-	snapshot, applied, err := applyWindowsACLPathGroup(windowsACLPathGroup{
-		Path: everyoneDir,
-		Entries: []WindowsACLEntry{{
-			Action:     WindowsACLAllowWrite,
-			Path:       everyoneDir,
-			Capability: "S-1-1-0",
-		}},
-	})
-	if err != nil {
-		t.Fatalf("grant Everyone write: %v", err)
-	}
-	if !applied {
-		t.Fatal("precondition: the Everyone grant did not apply, so there is no bypass to test for")
-	}
-	t.Cleanup(func() { _ = rollbackWindowsACLSnapshots([]windowsACLSnapshot{snapshot}) })
-
-	sandboxHome := filepath.Join(root, ".zero-sandbox")
-	// No DenyRead on purpose: that is what makes the runner choose the
-	// WRITE_RESTRICTED token, which is the default posture and the one whose
-	// restricted-SID list this test is about.
-	profile := PermissionProfile{
-		FileSystem: FileSystemPolicy{
-			Kind:                 FileSystemRestricted,
-			ReadRoots:            []string{root},
-			WriteRoots:           []WritableRoot{{Root: root, ProtectedMetadataNames: []string{".git", ".zero", ".agents"}}},
-			IncludePlatformRoots: true,
-			AllowTemp:            true,
-		},
-		Network: NetworkPolicy{Mode: NetworkDeny},
-	}
-	config := WindowsSandboxCommandArgsOptions{
-		SandboxHome:       sandboxHome,
-		CommandCWD:        root,
-		WorkspaceRoots:    []string{root},
-		PermissionProfile: profile,
-		SandboxLevel:      WindowsSandboxLevelUnelevated,
-	}
-
-	// Granted root first. A jail that denies everything would satisfy the two
-	// assertions below while being useless, and this separates the two outcomes.
-	insideMarker := filepath.Join(root, "write-ok.txt")
-	runWindowsRealSmokeCommand(t, runnerExe, config, []string{
-		"cmd.exe", "/d", "/s", "/c", "echo ok>" + insideMarker,
-	}, 0)
-	if contents, err := os.ReadFile(insideMarker); err != nil || strings.TrimSpace(string(contents)) != "ok" {
-		t.Fatalf("sandboxed write to a granted root = %q, %v; want ok", contents, err)
-	}
-
-	// Control: an ordinary directory outside every granted root. This is the
-	// behaviour the Everyone case must match.
-	controlMarker := filepath.Join(outside, "control-denied.txt")
-	runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(controlMarker), deniedWriteExitCode)
-	if _, err := os.Stat(controlMarker); err == nil {
-		t.Fatal("precondition: the sandbox allowed a write to an ordinary path outside every granted root, so this test cannot measure the Everyone case")
-	}
-
-	// The assertion. Same as the control in every respect except the DACL.
-	everyoneMarker := filepath.Join(everyoneDir, "everyone-denied.txt")
-	runWindowsRealSmokeCommand(t, runnerExe, config, deniedWriteCommand(everyoneMarker), deniedWriteExitCode)
-	if _, err := os.Stat(everyoneMarker); err == nil {
-		t.Error("the sandbox wrote outside every granted root because the path grants Everyone write; the restricted-SID list is satisfied by a SID every principal carries")
-	} else if !os.IsNotExist(err) {
-		t.Errorf("stat the Everyone-writable marker: %v", err)
-	}
-}
-
-// deniedWriteExitCode is chosen so a denial cannot be confused with the runner
-// failing to start the command at all.
-//
-// The obvious spelling of these assertions is `echo leaked>path` expecting exit
-// 1, but the runner itself exits 1 for its own errors — a failed marker
-// validation, a bad argument, a token it could not build. A test written that
-// way passes both when the sandbox denies the write and when nothing ever ran,
-// and the second case proves nothing while looking identical to success. On a
-// test whose whole job is catching a confinement regression, that is a failure
-// mode to design out rather than hope about.
-//
-// 77 is arbitrary beyond being outside the range the runner produces for itself.
-const deniedWriteExitCode = 77
-
-// deniedWriteCommand attempts a write and reports deniedWriteExitCode when the
-// redirect is refused, so the exit code also proves cmd.exe actually ran.
-func deniedWriteCommand(marker string) []string {
-	return []string{"cmd.exe", "/d", "/s", "/c", "echo leaked>" + marker + " || exit " + strconv.Itoa(deniedWriteExitCode)}
-}
-
 func powershellSingleQuote(value string) string {
 	out := "'"
 	for _, r := range value {

internal/sandbox/windows_token_windows.go

@@ -88,55 +88,19 @@ func createWindowsRestrictedTokenFromBase(base windows.Token, capabilitySIDs []w
 	if err != nil {
 		return 0, err
 	}
+	worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
+	if err != nil {
+		return 0, fmt.Errorf("create world SID: %w", err)
+	}
+
 	entries := make([]windows.SIDAndAttributes, 0, len(capabilitySIDs)+2)
 	for _, sid := range capabilitySIDs {
 		entries = append(entries, windows.SIDAndAttributes{Sid: sid.sid})
 	}
-	entries = append(entries, windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)})
-
-	// The World SID (S-1-1-0, Everyone) is added ONLY to the token that does not
-	// carry WRITE_RESTRICTED, and putting it back unconditionally would reopen a
-	// write-jail bypass.
-	//
-	// A restricted SID is a key to every object whose DACL names it. That is why
-	// the runner refuses to add the user SID, Administrators or SYSTEM — "each
-	// has write access nearly everywhere". Everyone is the broadest of the lot:
-	// every principal carries it, so under WRITE_RESTRICTED the second
-	// (restricted-SID) check passes for free on any path whose DACL grants
-	// Everyone write, and confinement falls back to the ordinary user's own
-	// permissions — the exact boundary this token exists to be stricter than. It
-	// needs no privilege, no symlink and no race: an Everyone-writable directory
-	// is enough, and share roots opened with Everyone:F and loose installer ACLs
-	// supply them. It was present from the original sandbox baseline,
-	// uncommented, and under WRITE_RESTRICTED nothing depends on it, because that
-	// flag already exempts reads from the restricted-SID check.
-	//
-	// Without the flag it IS load-bearing and cannot simply be dropped. The
-	// restricted-SID check then applies to READS too, and default Windows DACLs
-	// grant BUILTIN\Users rather than anything in this list, so a token without
-	// Everyone cannot open cmd.exe — the process dies at launch with
-	// STATUS_ACCESS_DENIED (0xC0000022) before it runs anything. That path is
-	// only taken when the profile configures DenyRead, which is already the
-	// posture that trades capability for read-deny enforcement (#612).
-	//
-	// So the bypass survives for DenyRead profiles, deliberately and narrowly,
-	// rather than being traded for a sandbox that cannot start a command. Closing
-	// it there needs a different mechanism (a read-side grant that is not a
-	// universal group), tracked separately.
-	//
-	// The logon SID above stays in both modes: it is this token's own rather than
-	// a broad group, and broadenWindowsRestrictedTokenDefaultDacl depends on it so
-	// the process can use pipes and events it creates for itself.
-	//
-	// Anything added here needs the same scrutiny — Authenticated Users, Users,
-	// INTERACTIVE and BATCH would each produce this bypass on a DACL naming them.
-	if !writeRestricted {
-		worldSID, err := windows.CreateWellKnownSid(windows.WinWorldSid)
-		if err != nil {
-			return 0, fmt.Errorf("create world SID: %w", err)
-		}
-		entries = append(entries, windows.SIDAndAttributes{Sid: worldSID})
-	}
+	entries = append(entries,
+		windows.SIDAndAttributes{Sid: sidFromBytes(logonSID)},
+		windows.SIDAndAttributes{Sid: worldSID},
+	)
 
 	// WRITE_RESTRICTED scopes the restricted-SID check to write-type accesses:
 	// reads use only the normal token identity, so the sandboxed process can

internal/specialist/exec.go

@@ -167,13 +167,10 @@ const permissionModeUnsafe = "unsafe"
 var readOnlySpecialistTools = map[string]bool{
 	"read_file":          true,
 	"read_minified_file": true,
-	// Reads one image file through read_file's own path scoping and mutates
-	// nothing, so it carries the same auto-approval as the other pure readers.
-	"view_image":     true,
-	"list_directory": true,
-	"grep":           true,
-	"glob":           true,
-	"update_plan":    true,
+	"list_directory":     true,
+	"grep":               true,
+	"glob":               true,
+	"update_plan":        true,
 }
 
 // IsReadOnlySpecialist reports whether the named specialist resolves to a

internal/specialist/exec_test.go

@@ -255,7 +255,7 @@ func TestBuildArgsDefaultsToReadOnlyToolAllowlist(t *testing.T) {
 	if err != nil {
 		t.Fatalf("BuildArgs returned error: %v", err)
 	}
-	if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file,view_image"}) {
+	if !containsSequence(result.Args, []string{"--enabled-tools", "glob,grep,list_directory,read_file,read_minified_file"}) {
 		t.Fatalf("args missing default read-only allowlist: %#v", result.Args)
 	}
 }

internal/specialist/manifest.go

@@ -82,9 +82,9 @@ var knownMetadataKeys = map[string]bool{
 }
 
 var toolCategories = map[string][]string{
-	"read-only": {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob"},
-	"edit":      {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
-	"execute":   {"read_file", "read_minified_file", "view_image", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
+	"read-only": {"read_file", "read_minified_file", "list_directory", "grep", "glob"},
+	"edit":      {"read_file", "read_minified_file", "list_directory", "grep", "glob", "write_file", "edit_file", "apply_patch"},
+	"execute":   {"read_file", "read_minified_file", "list_directory", "grep", "glob", "exec_command", "write_stdin", "bash"},
 	"plan":      {"update_plan"},
 }
 
@@ -102,7 +102,6 @@ var defaultToolSelection = []string{"read-only"}
 var knownToolNames = map[string]bool{
 	"read_file":           true,
 	"read_minified_file":  true,
-	"view_image":          true,
 	"list_directory":      true,
 	"glob":                true,
 	"grep":                true,

internal/specialist/view_image_category_test.go

@@ -1,31 +0,0 @@
-package specialist
-
-import (
-	"slices"
-	"testing"
-)
-
-// view_image is a pure workspace-scoped read, so every category that already
-// grants read_file should grant it too. Categories are what specialist authors
-// actually write ("tools: [execute]"), so an omission here silently denies a
-// specialist the ability to look at a screenshot it just captured.
-func TestToolCategoriesIncludeViewImageWhereverReadFileIsGranted(t *testing.T) {
-	for category, names := range toolCategories {
-		if !slices.Contains(names, "read_file") {
-			continue
-		}
-		if !slices.Contains(names, "view_image") {
-			t.Errorf("category %q grants read_file but not view_image", category)
-		}
-	}
-}
-
-func TestResolveToolsExpandsExecuteToIncludeViewImage(t *testing.T) {
-	resolved, err := ResolveTools([]string{"execute"})
-	if err != nil {
-		t.Fatalf("ResolveTools: %v", err)
-	}
-	if !slices.Contains(resolved, "view_image") {
-		t.Errorf("execute resolved to %v, missing view_image", resolved)
-	}
-}

internal/tools/args.go

@@ -132,34 +132,6 @@ func intPtr(value int) *int {
 	return &value
 }
 
-// sandboxPermissionsDescription and additionalPermissionsDescription are shared
-// by every shell tool (bash, exec_command) so the two fields cannot drift apart.
-//
-// Both lead with "omit" on purpose. The pair used to be described only in terms
-// of what each value does, which left a model that had used
-// with_additional_permissions once (say, for `gh issue create`) no instruction
-// to stop attaching a permission object to the next, ordinary command. That
-// attachment is then rejected — correctly, but the rejection alone does not tell
-// the model that dropping the field is the fix, so it flips between an
-// unflagged permission object and a flagged empty one and never converges
-// (#845, #847). Making "omit it" the first sentence of both descriptions stops
-// the invalid shape from being emitted in the first place; the validator errors
-// are the backstop, not the teacher.
-//
-// These ride on every turn's exec_command schema, so they are kept close to the
-// length of the text they replaced; TestEagerToolSchemaTokenBudget is the ratchet.
-const (
-	sandboxPermissionsDescription = "Per-command sandbox override. Omit it unless the command needs access the " +
-		"sandbox does not already give it; use `with_additional_permissions` with a non-empty " +
-		"`additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command " +
-		"must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by " +
-		"sandbox namespaces."
-
-	additionalPermissionsDescription = "Extra sandbox access. Omit this field entirely unless `sandbox_permissions` " +
-		"is `with_additional_permissions`; sending it with any other mode is rejected, as is an empty or all-false " +
-		"object. Example: {\"network\": {\"enabled\": true}}."
-)
-
 func additionalPermissionsProperties() map[string]PropertySchema {
 	return requestPermissionsProfileSchema().Properties
 }

internal/tools/bash.go

@@ -50,10 +50,10 @@ func NewScopedBashTool(workspaceRoot string, scope PathScope) Tool {
 					"command":             {Type: "string", Description: "Shell command to execute using the host shell. " + shellGuidance},
 					"cwd":                 {Type: "string", Description: "Directory to run the command in. Relative paths stay in the workspace; use an absolute path to run in a granted extra directory. Defaults to workspace root. Prefer cwd over cd when changing directories.", Default: "."},
 					"timeout_ms":          {Type: "integer", Description: "Command timeout in milliseconds.", Default: defaultBashTimeoutMS, Minimum: intPtr(1), Maximum: intPtr(maxBashTimeoutMS)},
-					"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
+					"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
 					"additional_permissions": {
 						Type:        "object",
-						Description: additionalPermissionsDescription,
+						Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
 						Properties:  additionalPermissionsProperties(),
 					},
 					"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},

internal/tools/exec_command.go

@@ -91,10 +91,10 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
 					"cwd":                 {Type: "string", Description: "Alias for workdir. Prefer workdir.", Default: "."},
 					"yield_time_ms":       {Type: "integer", Description: "Wait before yielding output. Defaults to 10000 ms; effective range is 250-30000 ms.", Default: defaultExecYieldTimeMS, Minimum: intPtr(1), Maximum: intPtr(maxExecYieldTimeMS)},
 					"max_output_tokens":   {Type: "integer", Description: "Output token budget. Defaults to 10000 tokens; larger requests may be capped by policy.", Default: defaultMaxOutputTokens, Minimum: intPtr(1), Maximum: intPtr(maxExecOutputTokenRequest)},
-					"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: sandboxPermissionsDescription, Default: string(SandboxPermissionsUseDefault)},
+					"sandbox_permissions": {Type: "string", Enum: []string{string(SandboxPermissionsUseDefault), string(SandboxPermissionsWithAdditionalPermissions), string(SandboxPermissionsRequireEscalated)}, Description: "Per-command sandbox override. Defaults to `use_default`; use `with_additional_permissions` with `additional_permissions` for sandboxed file/network access, or `require_escalated` only when the command must run outside the sandbox, such as host/global process, socket, service, or desktop state hidden by sandbox namespaces.", Default: string(SandboxPermissionsUseDefault)},
 					"additional_permissions": {
 						Type:        "object",
-						Description: additionalPermissionsDescription,
+						Description: "Sandboxed filesystem or network access for this command; only with `sandbox_permissions: \"with_additional_permissions\"`.",
 						Properties:  additionalPermissionsProperties(),
 					},
 					"justification": {Type: "string", Description: "User-facing approval question for `require_escalated`; omit otherwise."},

internal/tools/file_safety_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"os"
 	"path/filepath"
-	"strconv"
 	"strings"
 	"testing"
 )
@@ -296,40 +295,6 @@ func TestEditFileAllowsSeenRangeAndRejectsUnseenRange(t *testing.T) {
 	}
 }
 
-func TestCanonicalLimitedReadAuthorizesEdit(t *testing.T) {
-	dir := t.TempDir()
-	var source strings.Builder
-	for line := 1; line <= 1000; line++ {
-		source.WriteString("line ")
-		source.WriteString(strconv.Itoa(line))
-		source.WriteByte('\n')
-	}
-	path := filepath.Join(dir, "f.txt")
-	if err := os.WriteFile(path, []byte(source.String()), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	registry := safetyRegistry(t, dir)
-	tracker := NewFileTracker()
-	opts := grantedOpts(tracker)
-
-	read := registry.RunWithOptions(context.Background(), "read_file", map[string]any{
-		"path": "f.txt", "offset": 500, "limit": 10,
-	}, opts)
-	if read.Status != StatusOK || read.Truncated {
-		t.Fatalf("bounded canonical read must be exact: status=%s truncated=%v meta=%#v", read.Status, read.Truncated, read.Meta)
-	}
-	if read.Meta["seen_lines"] != "500-509" {
-		t.Fatalf("bounded canonical read did not receive exact-read credit: %#v", read.Meta)
-	}
-
-	edit := registry.RunWithOptions(context.Background(), "edit_file", map[string]any{
-		"path": "f.txt", "old_string": "line 505", "new_string": "updated 505",
-	}, opts)
-	if edit.Status != StatusOK {
-		t.Fatalf("edit inside canonical read range was refused: %s", edit.Output)
-	}
-}
-
 func TestEditFileReplaceAllRecordsEveryShiftedReplacementSpan(t *testing.T) {
 	dir := t.TempDir()
 	path := filepath.Join(dir, "f.txt")

internal/tools/file_tools_test.go

@@ -38,108 +38,6 @@ func TestReadFileToolReadsLineRanges(t *testing.T) {
 	}
 }
 
-func TestReadFileToolReadsCanonicalLineRange(t *testing.T) {
-	root := t.TempDir()
-	writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
-
-	result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
-		"path":   "notes.txt",
-		"offset": 2,
-		"limit":  2,
-	})
-
-	if result.Status != StatusOK {
-		t.Fatalf("expected ok status, got %s: %s", result.Status, result.Output)
-	}
-	if result.Truncated {
-		t.Fatalf("canonical limit defines the requested range and must not mark it truncated: %#v", result.Meta)
-	}
-	if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
-		t.Fatalf("canonical range returned the wrong lines: %q", result.Output)
-	}
-	if strings.Contains(result.Output, "alpha") || strings.Contains(result.Output, "delta") {
-		t.Fatalf("canonical range leaked outside requested slice: %q", result.Output)
-	}
-}
-
-func TestReadFileToolCanonicalOffsetPastEnd(t *testing.T) {
-	root := t.TempDir()
-	writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta")
-
-	result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
-		"path": "notes.txt", "offset": 10,
-	})
-
-	if result.Status != StatusOK || !strings.Contains(result.Output, "offset 10 is past the end") {
-		t.Fatalf("expected canonical out-of-range message, got status=%s output=%q", result.Status, result.Output)
-	}
-	if strings.Contains(result.Output, "start_line") {
-		t.Fatalf("out-of-range message exposed legacy argument: %q", result.Output)
-	}
-}
-
-func TestReadFileToolMixedLegacyRangesPreferLines(t *testing.T) {
-	root := t.TempDir()
-	writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
-
-	result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
-		"path":        "notes.txt",
-		"start_line":  2,
-		"end_line":    3,
-		"max_lines":   2,
-		"byte_offset": 0,
-		"byte_limit":  readFileByteChunkMax,
-	})
-
-	if result.Status != StatusOK {
-		t.Fatalf("mixed legacy range should recover, got %s: %s", result.Status, result.Output)
-	}
-	if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
-		t.Fatalf("mixed legacy range did not prefer lines: %q", result.Output)
-	}
-}
-
-func TestReadFileToolCombinesLegacyStartWithCanonicalLimit(t *testing.T) {
-	root := t.TempDir()
-	writeTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\ngamma\ndelta")
-
-	result := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
-		"path": "notes.txt", "start_line": 2, "limit": 2,
-	})
-
-	if result.Status != StatusOK || result.Truncated {
-		t.Fatalf("mixed compatible range should be exact, got status=%s truncated=%v: %s", result.Status, result.Truncated, result.Output)
-	}
-	if !strings.Contains(result.Output, "2 | beta") || !strings.Contains(result.Output, "3 | gamma") {
-		t.Fatalf("mixed compatible range returned the wrong lines: %q", result.Output)
-	}
-	if strings.Contains(result.Output, "alpha") || strings.Contains(result.Output, "delta") {
-		t.Fatalf("mixed compatible range leaked outside requested slice: %q", result.Output)
-	}
-}
-
-func TestFileToolSchemasExposeOnlyCanonicalArguments(t *testing.T) {
-	readProperties := NewScopedReadFileTool(t.TempDir(), nil).Parameters().Properties
-	for _, want := range []string{"path", "offset", "limit", "byte_offset", "byte_limit"} {
-		if _, ok := readProperties[want]; !ok {
-			t.Fatalf("read_file schema missing %q", want)
-		}
-	}
-	for _, legacy := range []string{"start_line", "end_line", "max_lines"} {
-		if _, ok := readProperties[legacy]; ok {
-			t.Fatalf("read_file schema must not expose legacy argument %q", legacy)
-		}
-	}
-
-	grepProperties := NewScopedGrepTool(t.TempDir(), nil).Parameters().Properties
-	if _, ok := grepProperties["case_insensitive"]; !ok {
-		t.Fatal("grep schema missing case_insensitive")
-	}
-	if _, ok := grepProperties["-i"]; ok {
-		t.Fatal("grep schema must not expose the -i alias")
-	}
-}
-
 func TestReadFileToolMarksTruncation(t *testing.T) {
 	root := t.TempDir()
 	writeTestFile(t, filepath.Join(root, "notes.txt"), "a\nb\nc\nd\ne")
@@ -154,7 +52,7 @@ func TestReadFileToolMarksTruncation(t *testing.T) {
 	}
 	// The cut must be visible in the rendered output, not just the Truncated flag,
 	// and must tell the model where to continue.
-	if !strings.Contains(result.Output, "[truncated:") || !strings.Contains(result.Output, "offset=3") {
+	if !strings.Contains(result.Output, "[truncated:") || !strings.Contains(result.Output, "start_line=3") {
 		t.Fatalf("expected truncation marker pointing to the next line, got %q", result.Output)
 	}
 }
@@ -170,7 +68,7 @@ func TestReadFileToolAppliesByteBudget(t *testing.T) {
 	if result.Status != StatusOK || !result.Truncated {
 		t.Fatalf("expected ok+truncated, got status=%s truncated=%v", result.Status, result.Truncated)
 	}
-	if !strings.Contains(result.Output, "output exceeded") || !strings.Contains(result.Output, "offset/limit") {
+	if !strings.Contains(result.Output, "output exceeded") || !strings.Contains(result.Output, "start_line") {
 		t.Fatalf("expected byte-budget continuation hint, got %q", result.Output[len(result.Output)-200:])
 	}
 	if result.Meta["raw_bytes"] == "" || result.Meta["emitted_bytes"] == "" {

internal/tools/file_tracker.go

@@ -377,5 +377,5 @@ func fileConflictMessage(relativePath string) string {
 }
 
 func fileUnseenMessage(relativePath string) string {
-	return "Error writing " + relativePath + ": the intended change depends on content that has not been read exactly in this session. Read the affected lines with read_file using offset/limit (or byte_offset/byte_limit for a very long line), then retry the edit."
+	return "Error writing " + relativePath + ": the intended change depends on content that has not been read exactly in this session. Read the affected lines with read_file, or use byte_offset/byte_limit for an oversized single line, then retry the edit."
 }

internal/tools/grep.go

@@ -48,6 +48,7 @@ func NewScopedGrepTool(workspaceRoot string, scope PathScope) Tool {
 					"path":             {Type: "string", Description: "Directory or file to search. Relative paths stay in the workspace; use an absolute path to search a granted extra root. Defaults to workspace root.", Default: "."},
 					"glob":             {Type: "string", Description: `Optional glob filter, for example "**/*.go".`},
 					"output_mode":      {Type: "string", Description: "Output mode.", Enum: []string{"content", "files_with_matches", "count"}, Default: "content"},
+					"-i":               {Type: "boolean", Description: "Case insensitive search.", Default: false},
 					"case_insensitive": {Type: "boolean", Description: "Case insensitive search.", Default: false},
 					"head_limit":       {Type: "integer", Description: "Maximum content results to return.", Default: 50, Minimum: intPtr(1)},
 				},

internal/tools/local_capture.go

@@ -4,8 +4,6 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
-	"github.com/Gitlawb/zero/internal/imageinput"
-	"github.com/Gitlawb/zero/internal/zeroruntime"
 	"os"
 	"path/filepath"
 	"regexp"
@@ -347,18 +345,9 @@ func captureOKResult(driver string, request captureArtifactRequest, path string,
 	if helperOutput != "" {
 		output += "\n\n" + helperOutput
 	}
-	// Attach the artifact itself. Without this the model is told a screenshot
-	// exists and cannot look at it, which is the whole point of asking for one.
-	// A PDF or an unreadable file simply yields no image and the text stands on
-	// its own, so a capture is never failed over the attachment.
-	var images []zeroruntime.ImageBlock
-	if image, err := imageinput.LoadFile(path, ""); err == nil {
-		images = []zeroruntime.ImageBlock{image}
-	}
 	return Result{
 		Status: StatusOK,
 		Output: output,
-		Images: images,
 		Meta: map[string]string{
 			"artifact_path": path,
 			"action":        request.action,

internal/tools/read_file.go

@@ -31,15 +31,16 @@ func NewScopedReadFileTool(workspaceRoot string, scope PathScope) Tool {
 	return readFileTool{
 		baseTool: baseTool{
 			name:        "read_file",
-			description: "Read exact file text with line numbers. Use for comments, formatting, or edits; prefer read_minified_file for initial code understanding. Use offset and limit for a line range.",
+			description: "Read a file by 1-based inclusive line range, or by exact byte chunks for oversized single-line files.",
 			parameters: Schema{
 				Type: "object",
 				Properties: map[string]PropertySchema{
-					"path":        {Type: "string", Description: "File path."},
-					"offset":      {Type: "integer", Description: "Optional 1-based source line to start from.", Minimum: intPtr(1)},
-					"limit":       {Type: "integer", Description: "Optional number of source lines to return.", Minimum: intPtr(1)},
-					"byte_offset": {Type: "integer", Description: "Optional zero-based offset for exact byte reads.", Minimum: intPtr(0)},
-					"byte_limit":  {Type: "integer", Description: "Optional byte count for an exact byte read; use with byte_offset.", Minimum: intPtr(1), Maximum: intPtr(readFileByteChunkMax)},
+					"path":        {Type: "string", Description: "Path of the file to read."},
+					"start_line":  {Type: "integer", Description: "1-based inclusive line number to start reading from.", Minimum: intPtr(1)},
+					"end_line":    {Type: "integer", Description: "1-based inclusive line number to stop reading at.", Minimum: intPtr(1)},
+					"max_lines":   {Type: "integer", Description: "Maximum number of lines to return.", Minimum: intPtr(1)},
+					"byte_offset": {Type: "integer", Description: "Zero-based byte offset for an exact chunk read. Use with byte_limit for oversized single-line files; cannot be combined with line ranges.", Minimum: intPtr(0)},
+					"byte_limit":  {Type: "integer", Description: "Maximum source bytes returned in exact byte mode. Defaults to 65536.", Default: readFileByteChunkMax, Minimum: intPtr(1), Maximum: intPtr(readFileByteChunkMax)},
 				},
 				Required:             []string{"path"},
 				AdditionalProperties: false,
@@ -68,27 +69,7 @@ func (tool readFileTool) run(args map[string]any, options RunOptions, directBudg
 	if err != nil {
 		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
 	}
-	_, hasOffset := args["offset"]
-	_, hasLimit := args["limit"]
-	_, hasStartLine := args["start_line"]
-	_, hasEndLine := args["end_line"]
-	_, hasMaxLines := args["max_lines"]
-	if hasOffset && hasStartLine {
-		return errorResult("Error: Invalid arguments for read_file: use either offset or start_line, not both")
-	}
-	if hasLimit && (hasEndLine || hasMaxLines) {
-		return errorResult("Error: Invalid arguments for read_file: use limit instead of end_line or max_lines, not both")
-	}
-
-	startKey := "offset"
-	if !hasOffset && hasStartLine {
-		startKey = "start_line"
-	}
-	startLine, err := intArg(args, startKey, 1, 1, 0)
-	if err != nil {
-		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
-	}
-	limit, err := intArg(args, "limit", 0, 1, 0)
+	startLine, err := intArg(args, "start_line", 1, 1, 0)
 	if err != nil {
 		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
 	}
@@ -100,25 +81,19 @@ func (tool readFileTool) run(args map[string]any, options RunOptions, directBudg
 	if err != nil {
 		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
 	}
-	if hasLimit {
-		const maxInt = int(^uint(0) >> 1)
-		endLine = startLine + min(limit-1, maxInt-startLine)
-	}
-	hasLineRange := hasOffset || hasLimit || hasStartLine || hasEndLine || hasMaxLines
 	_, hasByteOffset := args["byte_offset"]
 	_, hasByteLimit := args["byte_limit"]
-	byteMode := (hasByteOffset || hasByteLimit) && !hasLineRange
-	byteOffset := 0
-	byteLimit := readFileByteChunkMax
-	if byteMode {
-		byteOffset, err = intArg(args, "byte_offset", 0, 0, 0)
-		if err != nil {
-			return errorResult("Error: Invalid arguments for read_file: " + err.Error())
-		}
-		byteLimit, err = intArg(args, "byte_limit", readFileByteChunkMax, 1, readFileByteChunkMax)
-		if err != nil {
-			return errorResult("Error: Invalid arguments for read_file: " + err.Error())
-		}
+	byteMode := hasByteOffset || hasByteLimit
+	byteOffset, err := intArg(args, "byte_offset", 0, 0, 0)
+	if err != nil {
+		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
+	}
+	byteLimit, err := intArg(args, "byte_limit", readFileByteChunkMax, 1, readFileByteChunkMax)
+	if err != nil {
+		return errorResult("Error: Invalid arguments for read_file: " + err.Error())
+	}
+	if byteMode && (args["start_line"] != nil || args["end_line"] != nil || args["max_lines"] != nil) {
+		return errorResult("Error: Invalid arguments for read_file: byte_offset/byte_limit cannot be combined with line ranges")
 	}
 
 	absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
@@ -194,7 +169,7 @@ func renderedReadRange(total, start, end, maxLines int) (int, int) {
 
 func renderReadFileRange(absolutePath string, relativePath string, total int, startLine int, endLine int, maxLines int, maxBytes int) Result {
 	if startLine > total {
-		return okResult(fmt.Sprintf("File: %s\n(offset %d is past the end of the file, which has %d lines)", relativePath, startLine, total))
+		return okResult(fmt.Sprintf("File: %s\n(start_line %d is past the end of the file, which has %d lines)", relativePath, startLine, total))
 	}
 	if endLine == 0 || endLine > total {
 		endLine = total
@@ -233,9 +208,9 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
 	// Tool.Run calls preserve the legacy prefix-only byte budget.
 	var budgetedOutput *outputBudgetBuilder
 	if maxBytes > 0 {
-		budgetedOutput = newOutputBudgetBuilder(maxBytes, "use offset/limit to request a smaller line range")
+		budgetedOutput = newOutputBudgetBuilder(maxBytes, "use start_line/end_line or max_lines for normal files; use byte_offset/byte_limit for an oversized single line")
 	} else {
-		budgetedOutput = newHeadTailOutputBudgetBuilder(readOutputBudgetBytes, "use offset/limit to request a smaller line range")
+		budgetedOutput = newHeadTailOutputBudgetBuilder(readOutputBudgetBytes, "use start_line/end_line or max_lines for normal files; use byte_offset/byte_limit for an oversized single line")
 	}
 	budgetedOutput.WriteString(header)
 	budgetedOutput.WriteString("\n")
@@ -249,9 +224,9 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
 	}
 	if truncated {
 		// The Truncated flag alone is invisible to the model in the rendered
-		// output, so it cannot tell a limit cut from a complete read. Make the
+		// output, so it cannot tell a max_lines cut from a complete read. Make the
 		// cut explicit and tell it how to continue.
-		budgetedOutput.WriteString(fmt.Sprintf("\n\n[truncated: %d more line(s) in the requested range not shown; set offset=%d to continue]", endLine-lastLine, lastLine+1))
+		budgetedOutput.WriteString(fmt.Sprintf("\n\n[truncated: %d more line(s) in the requested range not shown; set start_line=%d to continue]", endLine-lastLine, lastLine+1))
 	}
 
 	budgeted := budgetedOutput.Result()
@@ -266,7 +241,7 @@ func renderReadFileRange(absolutePath string, relativePath string, total int, st
 		if budgeted.Truncated {
 			meta["truncation_reason"] = "byte_budget"
 		} else {
-			meta["truncation_reason"] = "limit"
+			meta["truncation_reason"] = "max_lines"
 		}
 	}
 	return Result{

internal/tools/read_minified_file.go

@@ -28,13 +28,12 @@ func NewScopedReadMinifiedFileTool(workspaceRoot string, scope PathScope) Tool {
 	return readMinifiedFileTool{
 		baseTool: baseTool{
 			name:        "read_minified_file",
-			description: "Read source code in a token-efficient, language-aware form. Prefer this for initial understanding; use read_file for exact text or line numbers.",
+			description: "Read a source file in a dense, token-cheap form: comments and redundant whitespace removed, no line numbers. Use it to scan or understand code for far fewer tokens than read_file. For exact text, comments, line numbers, or before editing, use read_file instead.",
+			deferred:    true,
 			parameters: Schema{
 				Type: "object",
 				Properties: map[string]PropertySchema{
-					"path":   {Type: "string", Description: "Source file path."},
-					"offset": {Type: "integer", Description: "Optional 1-based source line to start from.", Minimum: intPtr(1)},
-					"limit":  {Type: "integer", Description: "Optional maximum number of source lines to minify.", Minimum: intPtr(1)},
+					"path": {Type: "string", Description: "Path of the file to read in minified form."},
 				},
 				Required:             []string{"path"},
 				AdditionalProperties: false,
@@ -60,14 +59,6 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
 	if err != nil {
 		return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
 	}
-	offset, err := intArg(args, "offset", 1, 1, 0)
-	if err != nil {
-		return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
-	}
-	limit, err := intArg(args, "limit", 0, 1, 0)
-	if err != nil {
-		return errorResult("Error: Invalid arguments for read_minified_file: " + err.Error())
-	}
 
 	absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
 	if err != nil {
@@ -84,19 +75,11 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
 	info, _ := os.Stat(absolutePath)
 	options.FileTracker.Record(absolutePath, content, info)
 
-	selected := selectSourceLines(content, offset, limit)
-	if selected.pastEnd {
-		return okResult(fmt.Sprintf("File: %s\n(offset %d is past the end of the file, which has %d lines)", relativePath, offset, selected.totalLines))
-	}
-	ranged := offset > 1 || limit > 0
-	result := minify.File(relativePath, selected.content)
-	if ranged {
-		result = minify.ContextualFragment(relativePath, content, selected.content, selected.startByte)
-	}
-	rawLines := lineCount(string(selected.content))
+	result := minify.File(relativePath, content)
+	rawLines := lineCount(string(content))
 	minLines := lineCount(result.Content)
 	pct := 0
-	if rawBytes := len(selected.content); rawBytes > 0 {
+	if rawBytes := len(content); rawBytes > 0 {
 		if saved := rawBytes - len(result.Content); saved > 0 {
 			pct = saved * 100 / rawBytes
 		}
@@ -106,15 +89,12 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
 	if result.Applied {
 		header = fmt.Sprintf("File: %s — minified %s view (comments stripped, no line numbers; %d→%d lines, ~%d%% fewer bytes). For exact text/comments or before editing, use read_file.",
 			relativePath, result.Language, rawLines, minLines, pct)
-	} else if ranged {
-		header = fmt.Sprintf("File: %s — safe ranged view (whitespace normalized, no line numbers; %d→%d lines; context-sensitive stripping disabled because the range may begin inside a multiline construct). For exact text, use read_file.",
-			relativePath, rawLines, minLines)
 	} else {
 		header = fmt.Sprintf("File: %s — whitespace-normalized view (no line numbers; %d→%d lines; full minification not available for this file type). For exact text, use read_file.",
 			relativePath, rawLines, minLines)
 	}
 
-	rawBytes := len(selected.content)
+	rawBytes := len(content)
 	compactBytes := len(result.Content)
 	savedTokens := 0
 	if savedBytes := rawBytes - compactBytes; savedBytes > 0 {
@@ -133,54 +113,11 @@ func (tool readMinifiedFileTool) run(args map[string]any, options RunOptions, di
 	meta["estimated_tokens_saved"] = strconv.Itoa(savedTokens)
 	toolResult := Result{Status: StatusOK, Output: output, Meta: meta}
 	if directBudget {
-		return applyLegacyByteBudgetToResult(toolResult, readOutputBudgetBytes, "use offset/limit to request a smaller source range, or read_file when exact text is required")
+		return applyLegacyByteBudgetToResult(toolResult, readOutputBudgetBytes, "use read_file with start_line/end_line or max_lines for normal files, or byte_offset/byte_limit for an oversized single line")
 	}
 	return toolResult
 }
 
-type sourceSelection struct {
-	content    []byte
-	startByte  int
-	totalLines int
-	pastEnd    bool
-}
-
-func selectSourceLines(content []byte, offset, limit int) sourceSelection {
-	if offset <= 1 && limit == 0 {
-		return sourceSelection{content: content, totalLines: sourceLineCount(content)}
-	}
-	lines := strings.Split(string(content), "\n")
-	totalLines := sourceLineCount(content)
-	start := offset - 1
-	if start >= totalLines {
-		return sourceSelection{startByte: len(content), totalLines: totalLines, pastEnd: true}
-	}
-	startByte := 0
-	for index := 0; index < start; index++ {
-		startByte += len(lines[index]) + 1
-	}
-	end := totalLines
-	if limit > 0 && limit < end-start {
-		end = start + limit
-	}
-	return sourceSelection{
-		content:    []byte(strings.Join(lines[start:end], "\n")),
-		startByte:  startByte,
-		totalLines: totalLines,
-	}
-}
-
-func sourceLineCount(content []byte) int {
-	if len(content) == 0 {
-		return 1
-	}
-	lines := strings.Count(string(content), "\n")
-	if content[len(content)-1] != '\n' {
-		lines++
-	}
-	return lines
-}
-
 // lineCount reports the number of newline-separated lines in s (an empty string
 // counts as 0 lines, matching how a reader perceives an empty file).
 func lineCount(s string) int {

internal/tools/read_minified_file_test.go

@@ -45,123 +45,6 @@ func TestReadMinifiedFileRejectsTraversal(t *testing.T) {
 	}
 }
 
-func TestReadMinifiedFileSelectsSourceLineRangeBeforeMinifying(t *testing.T) {
-	dir := t.TempDir()
-	src := "package demo\n\nfunc One() int { return 1 }\n\nfunc Two() int { return 2 }\n"
-	if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte(src), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
-		"path": "f.go", "offset": 5, "limit": 1,
-	})
-	if res.Status != StatusOK || !strings.Contains(res.Output, "func Two") || strings.Contains(res.Output, "func One") {
-		t.Fatalf("unexpected ranged compact read: status=%s\n%s", res.Status, res.Output)
-	}
-}
-
-func TestReadMinifiedFileRangesPreserveUnknownLexicalContext(t *testing.T) {
-	tests := []struct {
-		name string
-		path string
-		src  string
-		want []string
-	}{
-		{
-			name: "multiline string",
-			path: "snippet.py",
-			src:  "value = \"\"\"\n# literal text\nstill literal\n\"\"\"\nprint(value)\n",
-			want: []string{"# literal text", "still literal"},
-		},
-		{
-			name: "template literal",
-			path: "snippet.js",
-			src:  "const value = `\n// literal text\nstill literal\n`;\n",
-			want: []string{"// literal text", "still literal"},
-		},
-		{
-			name: "block comment",
-			path: "snippet.c",
-			src:  "/* open\ncomment body\n*/\nint live;\n",
-			want: []string{"comment body", "*/"},
-		},
-	}
-
-	for _, tc := range tests {
-		t.Run(tc.name, func(t *testing.T) {
-			dir := t.TempDir()
-			if err := os.WriteFile(filepath.Join(dir, tc.path), []byte(tc.src), 0o644); err != nil {
-				t.Fatal(err)
-			}
-			res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
-				"path": tc.path, "offset": 2, "limit": 2,
-			})
-			if res.Status != StatusOK || res.Meta["compacted"] != "false" {
-				t.Fatalf("ranged read must use conservative normalization: status=%s meta=%#v\n%s", res.Status, res.Meta, res.Output)
-			}
-			for _, want := range tc.want {
-				if !strings.Contains(res.Output, want) {
-					t.Fatalf("ranged read lost lexical content %q:\n%s", want, res.Output)
-				}
-			}
-		})
-	}
-}
-
-func TestReadMinifiedFileRangeInsideGoRawStringIsConservative(t *testing.T) {
-	dir := t.TempDir()
-	src := "package demo\nvar value = `first\nSECRET-MARKER-ONE\n// literal line\nlast`\n"
-	if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte(src), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
-		"path": "f.go", "offset": 3, "limit": 2,
-	})
-	if res.Status != StatusOK || res.Meta["compacted"] != "false" {
-		t.Fatalf("raw-string range must use conservative normalization: status=%s meta=%#v\n%s", res.Status, res.Meta, res.Output)
-	}
-	for _, want := range []string{"SECRET-MARKER-ONE", "// literal line"} {
-		if !strings.Contains(res.Output, want) {
-			t.Fatalf("raw-string range lost or rewrote %q:\n%s", want, res.Output)
-		}
-	}
-	if strings.Contains(res.Output, "SECRET - MARKER - ONE") {
-		t.Fatalf("raw-string contents were parsed as Go code:\n%s", res.Output)
-	}
-}
-
-func TestReadMinifiedFileCanonicalOffsetPastEnd(t *testing.T) {
-	dir := t.TempDir()
-	if err := os.WriteFile(filepath.Join(dir, "f.go"), []byte("package demo\nvar value = 1\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
-		"path": "f.go", "offset": 10,
-	})
-	if res.Status != StatusOK || !strings.Contains(res.Output, "offset 10 is past the end") {
-		t.Fatalf("expected canonical out-of-range response, got status=%s output=%q", res.Status, res.Output)
-	}
-}
-
-func TestReadMinifiedFileMaximumLimitDoesNotOverflow(t *testing.T) {
-	dir := t.TempDir()
-	if err := os.WriteFile(filepath.Join(dir, "notes.txt"), []byte("alpha\nbeta\ngamma\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	const maxInt = int(^uint(0) >> 1)
-
-	res := NewScopedReadMinifiedFileTool(dir, nil).Run(context.Background(), map[string]any{
-		"path": "notes.txt", "offset": 2, "limit": maxInt,
-	})
-	if res.Status != StatusOK {
-		t.Fatalf("maximum limit should return the remaining range without panicking: status=%s output=%q", res.Status, res.Output)
-	}
-	if !strings.Contains(res.Output, "beta") || !strings.Contains(res.Output, "gamma") || strings.Contains(res.Output, "alpha") {
-		t.Fatalf("maximum limit returned the wrong range: %q", res.Output)
-	}
-}
-
 func TestReadMinifiedFileAppliesByteBudget(t *testing.T) {
 	dir := t.TempDir()
 	if err := os.WriteFile(filepath.Join(dir, "large.txt"), []byte(strings.Repeat("0123456789abcdef\n", 9000)), 0o644); err != nil {

internal/tools/registry.go

@@ -297,11 +297,8 @@ func scrubResultSecrets(res Result) Result {
 
 func CoreReadOnlyToolsScoped(workspaceRoot string, scope PathScope) []Tool {
 	return []Tool{
-		NewScopedReadMinifiedFileTool(workspaceRoot, scope),
 		NewScopedReadFileTool(workspaceRoot, scope),
-		// view_image is read-only and shares read_file's path scoping, so it
-		// belongs with the other readers rather than behind a gate.
-		NewScopedViewImageTool(workspaceRoot, scope),
+		NewScopedReadMinifiedFileTool(workspaceRoot, scope),
 		NewScopedListDirectoryTool(workspaceRoot, scope),
 		NewScopedGlobTool(workspaceRoot, scope),
 		NewScopedGrepTool(workspaceRoot, scope),

internal/tools/registry_test.go

@@ -30,8 +30,8 @@ func tempDirOutsideDefaultTemp(t *testing.T) string {
 
 func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
 	toolset := CoreReadOnlyToolsScoped(t.TempDir(), nil)
-	if len(toolset) != 10 {
-		t.Fatalf("expected 10 core read-only tools, got %d", len(toolset))
+	if len(toolset) != 9 {
+		t.Fatalf("expected 9 core read-only tools, got %d", len(toolset))
 	}
 
 	seen := map[string]bool{}
@@ -71,11 +71,6 @@ func TestCoreReadOnlyToolsExposeSafeMetadata(t *testing.T) {
 	if !seen[RequestPermissionsToolName] {
 		t.Fatalf("expected %s in core read-only tools", RequestPermissionsToolName)
 	}
-	// By name as well as by count: the count alone still passes if one reader is
-	// swapped for another.
-	if !seen[ViewImageToolName] {
-		t.Fatalf("expected %s in core read-only tools", ViewImageToolName)
-	}
 }
 
 func TestRegistryAllReturnsToolsSortedByName(t *testing.T) {
@@ -523,24 +518,25 @@ func TestIsDeferralEligibleDecouplesFromDeferred(t *testing.T) {
 
 func TestOptionalBuiltinsUseDeferredDiscovery(t *testing.T) {
 	wantDeferred := map[string]bool{
-		"bash":             true,
-		"browser_action":   true,
-		"browser_click":    true,
-		"browser_connect":  true,
-		"browser_install":  true,
-		"browser_launch":   true,
-		"browser_open":     true,
-		"browser_press":    true,
-		"browser_snapshot": true,
-		"browser_type":     true,
-		"capture_artifact": true,
-		"desktop_action":   true,
-		"desktop_snapshot": true,
-		"desktop_windows":  true,
-		"lsp_navigate":     true,
-		"terminal_session": true,
-		"web_fetch":        true,
-		"web_search":       true,
+		"bash":               true,
+		"browser_action":     true,
+		"browser_click":      true,
+		"browser_connect":    true,
+		"browser_install":    true,
+		"browser_launch":     true,
+		"browser_open":       true,
+		"browser_press":      true,
+		"browser_snapshot":   true,
+		"browser_type":       true,
+		"capture_artifact":   true,
+		"desktop_action":     true,
+		"desktop_snapshot":   true,
+		"desktop_windows":    true,
+		"lsp_navigate":       true,
+		"read_minified_file": true,
+		"terminal_session":   true,
+		"web_fetch":          true,
+		"web_search":         true,
 	}
 	for _, tool := range BuiltinCatalog(t.TempDir()) {
 		if _, listed := wantDeferred[tool.Name()]; listed {
@@ -548,7 +544,7 @@ func TestOptionalBuiltinsUseDeferredDiscovery(t *testing.T) {
 				t.Errorf("%s should be deferred", tool.Name())
 			}
 			delete(wantDeferred, tool.Name())
-		} else if tool.Name() == "read_file" || tool.Name() == "read_minified_file" || tool.Name() == "grep" || tool.Name() == ExecCommandToolName || tool.Name() == "edit_file" {
+		} else if tool.Name() == "read_file" || tool.Name() == "grep" || tool.Name() == ExecCommandToolName || tool.Name() == "edit_file" {
 			if IsDeferred(tool) {
 				t.Errorf("essential tool %s must stay eager", tool.Name())
 			}

internal/tools/sandbox_permissions_schema_test.go

@@ -1,72 +0,0 @@
-package tools
-
-import (
-	"strings"
-	"testing"
-)
-
-// The cheapest fix for #845/#847 is that the invalid shape is never emitted.
-// A model only learns "omit it" from the schema it is shown, so both shell
-// tools must say so in the field descriptions themselves — the validator errors
-// are the backstop, and by the time they fire a turn has already been burned.
-func TestShellToolsAdvertiseOmittingSandboxPermissionFields(t *testing.T) {
-	root := t.TempDir()
-	for _, shell := range []struct {
-		name string
-		tool Tool
-	}{
-		{"bash", NewScopedBashTool(root, nil)},
-		{"exec_command", NewScopedExecCommandTool(root, nil, nil)},
-	} {
-		properties := shell.tool.Parameters().Properties
-
-		mode, ok := properties["sandbox_permissions"]
-		if !ok {
-			t.Fatalf("%s: sandbox_permissions property missing", shell.name)
-		}
-		if !strings.Contains(mode.Description, "Omit it") {
-			t.Errorf("%s: sandbox_permissions description = %q, want it to lead with omitting the field", shell.name, mode.Description)
-		}
-		// Deliberately NOT "read-only". An earlier revision said every read-only
-		// command could omit the override, and that is false: `curl` and `gh` are
-		// read-only and still need sandboxed network. Conditioning omission on
-		// whether the command needs access it does not already have is the claim
-		// that is actually true, and the one a model can act on without first
-		// having to decide what counts as read-only.
-		if !strings.Contains(mode.Description, "unless the command needs access") {
-			t.Errorf("%s: sandbox_permissions description = %q, want omission conditioned on needing extra access", shell.name, mode.Description)
-		}
-
-		extra, ok := properties["additional_permissions"]
-		if !ok {
-			t.Fatalf("%s: additional_permissions property missing", shell.name)
-		}
-		if !strings.Contains(extra.Description, "Omit this field entirely") {
-			t.Errorf("%s: additional_permissions description = %q, want an explicit instruction to omit the field", shell.name, extra.Description)
-		}
-		// Both invalid shapes from the issue must be named as invalid up front,
-		// so neither is emitted "to be safe".
-		if !strings.Contains(extra.Description, "with any other mode is rejected") {
-			t.Errorf("%s: additional_permissions description = %q, want it to say the unflagged shape is rejected", shell.name, extra.Description)
-		}
-		if !strings.Contains(extra.Description, "empty or all-false object") {
-			t.Errorf("%s: additional_permissions description = %q, want it to say an empty payload is rejected", shell.name, extra.Description)
-		}
-	}
-}
-
-// The two shell tools describe the same two fields; they were copy-pasted and
-// must not drift apart again, or a model would be taught different rules by
-// bash and exec_command in the same session.
-func TestShellToolsShareSandboxPermissionDescriptions(t *testing.T) {
-	root := t.TempDir()
-	bash := NewScopedBashTool(root, nil).Parameters().Properties
-	exec := NewScopedExecCommandTool(root, nil, nil).Parameters().Properties
-
-	for _, field := range []string{"sandbox_permissions", "additional_permissions"} {
-		if bash[field].Description != exec[field].Description {
-			t.Errorf("%s description differs between bash and exec_command:\n bash = %q\n exec = %q",
-				field, bash[field].Description, exec[field].Description)
-		}
-	}
-}

internal/tools/types.go

@@ -5,7 +5,6 @@ import (
 
 	"github.com/Gitlawb/zero/internal/execution"
 	"github.com/Gitlawb/zero/internal/sandbox"
-	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
 type SideEffect string
@@ -103,16 +102,6 @@ type Result struct {
 	// while callers migrate away from parsing text and string metadata.
 	ExecutionRequest *execution.Request `json:"-"`
 	ExecutionOutcome *execution.Outcome `json:"-"`
-	// Images are pictures the tool is handing the model — a screenshot it just
-	// captured, a file it was asked to look at. Text-only tools leave it nil.
-	//
-	// They are NOT delivered on this result's own tool-result message. Every
-	// provider drops images there: Anthropic maps a tool result to a tool_result
-	// block whose content is a string, Gemini to a functionResponse, and OpenAI
-	// guards its image content-parts to the user role. The agent loop therefore
-	// emits them as a following user message, which is also the only shape that
-	// keeps one tool result per tool call.
-	Images []zeroruntime.ImageBlock `json:"-"`
 	// Redacted is set when secret scrubbing altered Output before it left the
 	// tool-execution boundary.
 	Redacted bool

internal/tools/update_plan.go

@@ -26,7 +26,7 @@ func NewUpdatePlanTool() *updatePlanTool {
 	return &updatePlanTool{
 		baseTool: baseTool{
 			name: "update_plan",
-			description: "Create or update the in-memory plan for implementation or investigation with at least three meaningful dependent steps. Do not use it for simple lookups, explanations, or code navigation. " +
+			description: "Create or update the in-memory plan for the current task. " +
 				"Pass the full ordered list of steps each call; it replaces the previous plan. " +
 				"Each item needs a `content` string; `status` defaults to \"pending\" and `id` is " +
 				"auto-numbered, so you only need to supply `content` (and `status` as the task progresses). " +

internal/tools/view_image.go

@@ -1,80 +0,0 @@
-package tools
-
-import (
-	"context"
-
-	"github.com/Gitlawb/zero/internal/imageinput"
-	"github.com/Gitlawb/zero/internal/zeroruntime"
-)
-
-// ViewImageToolName is the canonical registry name of the image viewer.
-const ViewImageToolName = "view_image"
-
-// NewViewImageTool builds the workspace-scoped image viewer.
-func NewViewImageTool(workspaceRoot string) Tool {
-	return NewScopedViewImageTool(workspaceRoot, nil)
-}
-
-// NewScopedViewImageTool builds the image viewer bound to a path scope.
-//
-// The scope is not optional decoration. imageinput.LoadFile resolves a relative
-// path by joining it to the workspace root and otherwise takes the path as
-// given, so on its own it would happily read outside the workspace. Every path
-// goes through resolveScopedReadPath first, exactly like read_file, so this tool
-// grants no reach that read_file does not already have.
-func NewScopedViewImageTool(workspaceRoot string, scope PathScope) Tool {
-	return viewImageTool{
-		baseTool: baseTool{
-			name: ViewImageToolName,
-			description: "Look at an image file. Use this to see a screenshot, diagram, mockup or photo " +
-				"the user mentioned but did not attach, or one a previous tool wrote to disk. " +
-				"Accepts PNG, JPEG, GIF and WebP up to 10 MiB.",
-			parameters: Schema{
-				Type: "object",
-				Properties: map[string]PropertySchema{
-					"path": {Type: "string", Description: "Path of the image to look at."},
-				},
-				Required:             []string{"path"},
-				AdditionalProperties: false,
-			},
-			safety: readOnlySafety("Reads an image file without modifying anything."),
-			// Same shape as read_file: reads one file, mutates nothing, and is
-			// safe to run concurrently.
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: true, ResourceKeys: fileResourceKeys},
-		},
-		workspaceRoot: workspaceRoot,
-		scope:         scope,
-	}
-}
-
-type viewImageTool struct {
-	baseTool
-	workspaceRoot string
-	scope         PathScope
-}
-
-func (tool viewImageTool) Run(_ context.Context, args map[string]any) Result {
-	requestedPath, err := aliasedStringArg(args, []string{"path", "file", "file_path", "filepath", "filename", "image"}, "", true, false)
-	if err != nil {
-		return errorResult("Error: Invalid arguments for " + ViewImageToolName + ": " + err.Error())
-	}
-	absolutePath, relativePath, err := resolveScopedReadPath(tool.workspaceRoot, tool.scope, requestedPath)
-	if err != nil {
-		return errorResult("Error: " + err.Error())
-	}
-	image, err := imageinput.LoadFile(absolutePath, tool.workspaceRoot)
-	if err != nil {
-		// LoadFile's errors already name the problem (missing, too large,
-		// unsupported type) and are safe to show: they carry the path the caller
-		// asked for, which it already knows.
-		return errorResult("Error viewing " + relativePath + ": " + err.Error())
-	}
-	return Result{
-		Status: StatusOK,
-		// The text says what arrived; the image itself reaches the model through
-		// the agent loop, which delivers Images on a following user message.
-		Output: "Viewing " + relativePath + " (" + image.MediaType + ").",
-		Images: []zeroruntime.ImageBlock{image},
-		Meta:   map[string]string{"media_type": image.MediaType},
-	}
-}

internal/tools/view_image_test.go

@@ -1,100 +0,0 @@
-package tools
-
-import (
-	"context"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-)
-
-// A 1x1 PNG. Small enough to inline, real enough to pass media-type sniffing.
-var onePixelPNG = []byte{
-	0x89, 'P', 'N', 'G', 0x0d, 0x0a, 0x1a, 0x0a,
-	0x00, 0x00, 0x00, 0x0d, 'I', 'H', 'D', 'R',
-	0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x01,
-	0x08, 0x06, 0x00, 0x00, 0x00, 0x1f, 0x15, 0xc4,
-	0x89, 0x00, 0x00, 0x00, 0x0a, 'I', 'D', 'A', 'T',
-	0x78, 0x9c, 0x63, 0x00, 0x01, 0x00, 0x00, 0x05,
-	0x00, 0x01, 0x0d, 0x0a, 0x2d, 0xb4, 0x00, 0x00,
-	0x00, 0x00, 'I', 'E', 'N', 'D', 0xae, 0x42, 0x60, 0x82,
-}
-
-func TestViewImageReturnsTheImage(t *testing.T) {
-	root := t.TempDir()
-	if err := os.WriteFile(filepath.Join(root, "shot.png"), onePixelPNG, 0o600); err != nil {
-		t.Fatal(err)
-	}
-	result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "shot.png"})
-
-	if result.Status != StatusOK {
-		t.Fatalf("status = %v: %s", result.Status, result.Output)
-	}
-	if len(result.Images) != 1 {
-		t.Fatalf("got %d images, want 1 — the text alone is useless to the model", len(result.Images))
-	}
-	if result.Images[0].MediaType != "image/png" {
-		t.Errorf("media type = %q, want image/png", result.Images[0].MediaType)
-	}
-	if len(result.Images[0].Data) != len(onePixelPNG) {
-		t.Errorf("image data is %d bytes, want %d", len(result.Images[0].Data), len(onePixelPNG))
-	}
-}
-
-// The tool must not reach outside the workspace. imageinput.LoadFile does no
-// containment check of its own, so the scoping is entirely this tool's job.
-func TestViewImageRefusesPathsOutsideTheWorkspace(t *testing.T) {
-	root := t.TempDir()
-	outside := t.TempDir()
-	secret := filepath.Join(outside, "outside.png")
-	if err := os.WriteFile(secret, onePixelPNG, 0o600); err != nil {
-		t.Fatal(err)
-	}
-	tool := NewViewImageTool(root)
-
-	for name, path := range map[string]string{
-		"absolute path outside": secret,
-		"parent traversal":      filepath.Join("..", filepath.Base(outside), "outside.png"),
-	} {
-		t.Run(name, func(t *testing.T) {
-			result := tool.Run(context.Background(), map[string]any{"path": path})
-			if result.Status == StatusOK {
-				t.Fatalf("read an image outside the workspace: %s", result.Output)
-			}
-			if len(result.Images) != 0 {
-				t.Error("a refused read still returned image bytes")
-			}
-		})
-	}
-}
-
-func TestViewImageRejectsNonImages(t *testing.T) {
-	root := t.TempDir()
-	if err := os.WriteFile(filepath.Join(root, "notes.txt"), []byte("not an image"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-	result := NewViewImageTool(root).Run(context.Background(), map[string]any{"path": "notes.txt"})
-	if result.Status == StatusOK {
-		t.Fatalf("accepted a text file as an image: %s", result.Output)
-	}
-	if len(result.Images) != 0 {
-		t.Error("a rejected file still returned image bytes")
-	}
-}
-
-func TestViewImageReportsAMissingFileClearly(t *testing.T) {
-	result := NewViewImageTool(t.TempDir()).Run(context.Background(), map[string]any{"path": "nope.png"})
-	if result.Status == StatusOK {
-		t.Fatal("a missing file reported success")
-	}
-	// The error names the path the caller asked for, matching read_file. Both
-	// also carry the resolved absolute path from the underlying syscall error;
-	// that is pre-existing behaviour shared with read_file, not something this
-	// tool introduces, so it is not asserted either way here.
-	if !strings.Contains(result.Output, "nope.png") {
-		t.Errorf("error should name the file, got %q", result.Output)
-	}
-	if len(result.Images) != 0 {
-		t.Error("a failed read still returned image bytes")
-	}
-}

internal/tui/clipboard_nul_test.go

@@ -1,60 +0,0 @@
-package tui
-
-import (
-	"strings"
-	"testing"
-
-	"github.com/charmbracelet/x/ansi"
-)
-
-// A COPIED NUL KILLED THE WHOLE PROGRAM ON WINDOWS.
-//
-// github.com/atotto/clipboard's Windows path calls syscall.StringToUTF16, which
-// PANICS rather than returning an error when the string contains a NUL — so
-// selecting transcript text that carried one took the TUI down with "program
-// experienced a panic" and a stack ending in clipboard.writeAll. A teammate hit
-// it on a real Windows machine.
-//
-// The NUL reaches the clipboard because ansi.Strip removes escape SEQUENCES and
-// leaves C0 bytes alone, and the transcript carries them from two directions:
-// tool output (binary reads, git's -z listings) and this package's own card
-// protocol, whose prefixes are literally "\x00command-card\x00".
-func TestNulNeverReachesTheClipboard(t *testing.T) {
-	// The card prefix is a real, in-repo source: ansi.Strip leaves it intact.
-	if !strings.ContainsRune(ansi.Strip(commandCardTranscriptPrefix+"Profile"), 0) {
-		t.Fatal("setup: the card prefix no longer carries a NUL; this test guards the wrong thing")
-	}
-	for _, tc := range []struct{ name, in string }{
-		{"card prefix", commandCardTranscriptPrefix + "Profile\nstatus: ok"},
-		{"plan card prefix", planCardTranscriptPrefix + "plan"},
-		{"binary tool output", "before\x00after"},
-		{"git -z listing", "a.go\x00b.go\x00c.go"},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			got := clipboardSafeText(tc.in)
-			if strings.ContainsRune(got, 0) {
-				t.Fatalf("a NUL survived to the clipboard write: %q — Windows would panic here", got)
-			}
-			// The readable content must survive: this sanitises, it does not truncate.
-			for _, want := range strings.Split(strings.ReplaceAll(tc.in, "\x00", "\n"), "\n") {
-				if want = strings.TrimSpace(want); want != "" && !strings.Contains(got, want) {
-					t.Fatalf("sanitising dropped real content %q from %q", want, got)
-				}
-			}
-		})
-	}
-}
-
-// Newlines and tabs are legal clipboard content and must survive — a multi-line
-// selection is the normal case, not the exception.
-func TestClipboardSanitisingKeepsLayout(t *testing.T) {
-	in := "line one\n\tindented\nline three"
-	if got := clipboardSafeText(in); got != in {
-		t.Fatalf("clean multi-line text was altered:\n got %q\nwant %q", got, in)
-	}
-	// Other C0 bytes go: they would terminate the OSC52 escape early and
-	// corrupt the copy even where they do not panic.
-	if got := clipboardSafeText("a\x07b\x1bc"); got != "abc" {
-		t.Fatalf("control bytes survived: %q", got)
-	}
-}

internal/tui/transcript_selection.go

@@ -1499,54 +1499,7 @@ func (m model) selectedTranscriptText() string {
 	return strings.Join(parts, "\n")
 }
 
-// clipboardSafeText removes the control characters a clipboard write cannot
-// carry, keeping newlines and tabs.
-//
-// A NUL PANICS THE WHOLE PROGRAM ON WINDOWS, and that is not a theoretical
-// concern: syscall.StringToUTF16 — which github.com/atotto/clipboard calls on
-// Windows — PANICS rather than erroring when the string contains one, so a
-// single copied NUL killed the TUI with "program experienced a panic". It
-// reached the clipboard because ansi.Strip removes escape SEQUENCES and leaves
-// C0 bytes untouched, and the transcript legitimately carries them from two
-// directions: tool output (a read of a binary file, git's -z NUL-separated
-// listings) and this package's own card protocol, whose row prefixes are
-// literally "\x00command-card\x00".
-//
-// Stripped at the WRITE, not at each source, because the sources are unbounded:
-// any tool result a user can select from is a source, and a fix per source is
-// one that the next tool re-breaks. The same pass protects the OSC52 fallback,
-// where a stray control byte would terminate the escape sequence early and
-// corrupt the copy rather than crash.
-//
-// \n and \t survive: a multi-line selection is the normal case, and both are
-// legal clipboard content.
-func clipboardSafeText(text string) string {
-	if !strings.ContainsFunc(text, unsafeClipboardRune) {
-		return text
-	}
-	var out strings.Builder
-	out.Grow(len(text))
-	for _, r := range text {
-		if unsafeClipboardRune(r) {
-			continue
-		}
-		out.WriteRune(r)
-	}
-	return out.String()
-}
-
-func unsafeClipboardRune(r rune) bool {
-	if r == '\n' || r == '\t' {
-		return false
-	}
-	return r < 0x20 || r == 0x7f
-}
-
 func copyTranscriptSelectionCmd(text string) tea.Cmd {
-	// SANITISED ONCE, HERE, before either destination sees it — the native
-	// clipboard would panic on a NUL and the OSC52 fallback would be corrupted
-	// by one.
-	text = clipboardSafeText(text)
 	return func() tea.Msg {
 		// Prefer the native OS clipboard (pbcopy / clip.exe / xclip): it works on
 		// local terminals — including macOS Terminal.app, which has no OSC52 support