Trim eager read tool schemas

* docs(agents): codify recurring review blockers in AGENTS.md

Capture the high-frequency classes that drive multi-round maintainer reviews
(stale bases, path canonicalization, secret handling, partial setup, missing
regression tests, scope honesty) so coding agents fix them before first review.

Refs #856

* docs(agents): tighten security and atomic-write review guidance

Require open-time containment binding instead of pre-open EvalSymlinks, and
require temp-file then atomic replace rather than exclusive create alone.

Refs #856

* docs(agents): require traversal-wide containment for no-follow APIs

Final-component-only O_NOFOLLOW is insufficient on Linux and reparse
points need platform protections. Align the Security edges guidance
with the full-path containment requirement from review.

.gitattributes

@@ -0,0 +1,3 @@
+*.go text eol=lf
+*.mod text eol=lf
+*.sum text eol=lf

.github/workflows/ci.yml

@@ -30,6 +30,36 @@ jobs:
           go-version-file: go.mod
           cache: true
 
+      - name: Install Make on Windows
+        if: matrix.os == 'windows-latest'
+        shell: pwsh
+        run: choco install make --no-progress -y
+
+      - name: Validate quality targets with stock macOS Make
+        if: matrix.os == 'macos-latest'
+        env:
+          GIT_CONFIG_COUNT: 1
+          GIT_CONFIG_KEY_0: grep.patternType
+          GIT_CONFIG_VALUE_0: fixed
+        run: make -n vulncheck
+
+      - name: Check vulnerabilities through native Windows Make
+        if: matrix.os == 'windows-latest'
+        shell: cmd
+        run: make vulncheck
+
+      - name: Check dead code through native Windows Make
+        if: matrix.os == 'windows-latest'
+        continue-on-error: true
+        shell: cmd
+        run: make deadcode
+
+      - name: Run static lint through native Windows Make
+        if: matrix.os == 'windows-latest'
+        continue-on-error: true
+        shell: cmd
+        run: make lint-static
+
       - name: Check formatting
         if: matrix.os == 'ubuntu-latest'
         shell: bash
@@ -100,27 +130,18 @@ jobs:
           go-version-file: go.mod
           cache: true
 
-      # govulncheck, deadcode, and golangci-lint each resolve a toolchain from
-      # their own modules, which can drop below the version go.mod requires and
-      # then fail to load our packages. Pin GOTOOLCHAIN to the go.mod toolchain
-      # so all three run under it.
-      - name: Pin toolchain from go.mod
-        run: |
-          toolchain="$(awk '/^toolchain /{print $2}' go.mod)"
-          echo "GOTOOLCHAIN=${toolchain:-auto}" >> "$GITHUB_ENV"
-
       # Hard gate: fails the build when code reaches a known vulnerability. A stdlib
       # CVE is cleared by a toolchain bump (see go.mod). May also flag a newly
       # published advisory on an unrelated PR — intentional: do not ship known vulns.
       - name: govulncheck
-        run: go run golang.org/x/vuln/cmd/[email protected] ./...
+        run: make vulncheck
 
       # Advisory: reports functions unreachable from any cmd/* main so dormant
       # code is visible in CI. Non-blocking while the dormant subsystems are
       # still being wired or removed.
       - name: deadcode (advisory)
         continue-on-error: true
-        run: go run golang.org/x/tools/cmd/[email protected] -test=false ./...
+        run: make deadcode
 
       # Advisory: catches what deadcode's whole-program reachability analysis
       # doesn't, unused private functions/assignments reachable within a
@@ -130,4 +151,4 @@ jobs:
       # the repo are cleaned up incrementally (see #527).
       - name: golangci-lint (advisory)
         continue-on-error: true
-        run: go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...
+        run: make lint-static

.release-please-manifest.json

@@ -1 +1 @@
-{".":"0.5.0"}
+{".":"0.6.0"}

AGENTS.md

@@ -51,10 +51,8 @@ request, or completing an implementation task:
    or run focused tests with `-race`, when concurrency is affected.
 4. **Build**: `go run ./cmd/zero-release build`.
 5. **Smoke test**: `go run ./cmd/zero-release smoke`.
-6. **Advisory lint**:
-   `go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...`.
-7. **Security**:
-   `go run golang.org/x/vuln/cmd/[email protected] ./...`.
+6. **Advisory lint**: `make lint-static`.
+7. **Security**: `make vulncheck`.
 8. **Diff hygiene**: `git diff HEAD --check` (covers staged and unstaged
    tracked changes).
 
@@ -69,3 +67,39 @@ 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.

CHANGELOG.md

@@ -6,6 +6,37 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 aims to follow [Semantic Versioning](https://semver.org/spec/v2.0.0.html) once the first release is
 tagged. Until then, source builds report the version `dev`.
 
+## [0.6.0](https://github.com/Gitlawb/zero/compare/v0.5.0...v0.6.0) (2026-07-29)
+
+
+### Features
+
+* add persistent session goals ([#803](https://github.com/Gitlawb/zero/issues/803)) ([5d1869e](https://github.com/Gitlawb/zero/commit/5d1869e7bc193c97e07418b57b823905c1151d5d))
+* **providers:** add Fireworks AI provider preset ([#772](https://github.com/Gitlawb/zero/issues/772)) ([58daea7](https://github.com/Gitlawb/zero/commit/58daea7a482927e5ab35afaf97761c60bb35e113))
+* **skills:** report hash drift in skill info ([#795](https://github.com/Gitlawb/zero/issues/795)) ([9fbdc34](https://github.com/Gitlawb/zero/commit/9fbdc341551601c13434cf7b9b6f5e4ea4c483b1))
+
+
+### Bug Fixes
+
+* **acp:** interrupt idle reads on cancellation ([#782](https://github.com/Gitlawb/zero/issues/782)) ([15b7a54](https://github.com/Gitlawb/zero/commit/15b7a5475cfeefddbc59634551451a8872b996b7))
+* **cli:** include plugins info in shell completions ([#794](https://github.com/Gitlawb/zero/issues/794)) ([fa8734f](https://github.com/Gitlawb/zero/commit/fa8734f17927de8832dfe041e043cfb86f93d12a))
+* **config:** kill provider-command process tree via job object on Windows ([#690](https://github.com/Gitlawb/zero/issues/690)) ([5c58256](https://github.com/Gitlawb/zero/commit/5c58256abc29c185b449f8ce79647eb3fae61882))
+* **config:** make the provider-command timeout an upper bound ([#811](https://github.com/Gitlawb/zero/issues/811)) ([#813](https://github.com/Gitlawb/zero/issues/813)) ([1c38e8c](https://github.com/Gitlawb/zero/commit/1c38e8cba50db255f3cda8b7cf4298712b2933e1))
+* **config:** stop billing provider-command fixtures to the 5s timeout ([#810](https://github.com/Gitlawb/zero/issues/810)) ([f5fa4ff](https://github.com/Gitlawb/zero/commit/f5fa4ffc64537897aa78a56edca36d8f1dacd021))
+* **cron:** reserve job IDs atomically ([#686](https://github.com/Gitlawb/zero/issues/686)) ([d9b882e](https://github.com/Gitlawb/zero/commit/d9b882eb0a94f38de8447cb54b57860ccd8ed053))
+* **lint:** resolve static analysis baseline ([#769](https://github.com/Gitlawb/zero/issues/769)) ([583e653](https://github.com/Gitlawb/zero/commit/583e6530bd849a8efcb2b88bbe8c297e8592e62c))
+* **providers:** retry provably pre-send transport failures ([#447](https://github.com/Gitlawb/zero/issues/447)) ([#750](https://github.com/Gitlawb/zero/issues/750)) ([8acd42a](https://github.com/Gitlawb/zero/commit/8acd42a3bd743a35807d762f23a33fb61c171839))
+* **sandbox:** add platform-specific write lock around grant state file ([#752](https://github.com/Gitlawb/zero/issues/752)) ([#755](https://github.com/Gitlawb/zero/issues/755)) ([88fb7b6](https://github.com/Gitlawb/zero/commit/88fb7b6192c1ac8173089967d5e9119f92d7a101))
+* **sandbox:** clarify scoped permission prompts ([#798](https://github.com/Gitlawb/zero/issues/798)) ([9fe02ae](https://github.com/Gitlawb/zero/commit/9fe02ae52e3234cb9dae76e0c5594a287ccbb52a))
+* **sandbox:** deny reads of git's credential stores ([#815](https://github.com/Gitlawb/zero/issues/815)) ([#816](https://github.com/Gitlawb/zero/issues/816)) ([33c94ec](https://github.com/Gitlawb/zero/commit/33c94ec7a3e7987e6e36b5fd5e037ff4e6e84c26))
+* **sandbox:** preserve user config and retry denied writes ([#801](https://github.com/Gitlawb/zero/issues/801)) ([81a5e6c](https://github.com/Gitlawb/zero/commit/81a5e6cb716ba4b7ecf7ff3ff261dfa51fd01098))
+* **sandbox:** scrub the daemon token file pointer ([#677](https://github.com/Gitlawb/zero/issues/677)) ([#818](https://github.com/Gitlawb/zero/issues/818)) ([3e7692d](https://github.com/Gitlawb/zero/commit/3e7692d8f0885d346809869935c81a9cf055d89e))
+* **swarm:** close lifecycle admission on shutdown ([#776](https://github.com/Gitlawb/zero/issues/776)) ([bdbda89](https://github.com/Gitlawb/zero/commit/bdbda89f3e42d93e8058ab3e1e9ef402659530cd))
+* **tui:** fix rendering corruption over multipass + Windows Terminal ([#709](https://github.com/Gitlawb/zero/issues/709)) ([5bb1bc1](https://github.com/Gitlawb/zero/commit/5bb1bc11c3188dcea7709ff07c8b25dda3541de8))
+* **tui:** paste clipboard images with Ctrl+V ([#534](https://github.com/Gitlawb/zero/issues/534)) ([#807](https://github.com/Gitlawb/zero/issues/807)) ([ec3b822](https://github.com/Gitlawb/zero/commit/ec3b8225050eb62f0f41a9582ed2881d2f9dcc8e))
+* **tui:** reset quiet clock for new runs ([#799](https://github.com/Gitlawb/zero/issues/799)) ([a4a805a](https://github.com/Gitlawb/zero/commit/a4a805a6c82c51cb255e909bd87ca183c8805cf2))
+* **windows:** use PowerShell for command execution ([#804](https://github.com/Gitlawb/zero/issues/804)) ([518d2e5](https://github.com/Gitlawb/zero/commit/518d2e53ba3fa073090a9b41c278face85b59b64))
+
 ## [0.5.0](https://github.com/Gitlawb/zero/compare/v0.4.0...v0.5.0) (2026-07-22)
 
 

Makefile

@@ -1,7 +1,13 @@
-# Zero build/test/lint targets. AGENTS.md says "Build with `make`" and "Run `make
-# lint` before opening a PR" — these targets back those instructions.
+# Zero build/test/lint targets. AGENTS.md says to build and run quality checks
+# with `make` — these targets back those instructions.
 .DEFAULT_GOAL := build
-.PHONY: build build-all test test-race vet fmt fmt-check lint tidy clean baseline help
+GO_VERSION = $(word 2,$(shell git grep -G -h "^go[[:space:]]" -- go.mod))
+GO_TOOLCHAIN = go$(GO_VERSION)
+DEADCODE_VERSION := v0.46.0
+GOLANGCI_LINT_VERSION := v2.12.2
+GOVULNCHECK_VERSION := v1.3.0
+
+.PHONY: build build-all test test-race vet fmt fmt-check lint lint-static deadcode vulncheck tidy clean baseline help
 
 # Build the main CLI binary into ./zero.
 build:
@@ -33,6 +39,22 @@ fmt-check:
 # Lint = formatting check + vet (no extra tooling required).
 lint: fmt-check vet
 
+# Versioned tools select the toolchain from their own modules when invoked with
+# package@version. Read this module's go directive directly, without invoking
+# the possibly stale Go toolchain or consulting a multi-module GOWORK. git grep
+# works with both POSIX shells and cmd.exe, including GNU Make 3.81 on macOS.
+# The target-specific export is shell-independent.
+lint-static deadcode vulncheck: export GOTOOLCHAIN = $(GO_TOOLCHAIN)
+
+lint-static:
+	go run github.com/golangci/golangci-lint/v2/cmd/golangci-lint@$(GOLANGCI_LINT_VERSION) run --enable-only unused,ineffassign,staticcheck ./...
+
+deadcode:
+	go run golang.org/x/tools/cmd/deadcode@$(DEADCODE_VERSION) -test=false ./...
+
+vulncheck:
+	go run golang.org/x/vuln/cmd/govulncheck@$(GOVULNCHECK_VERSION) ./...
+
 tidy:
 	go mod tidy
 
@@ -55,4 +77,4 @@ baseline: build
 		--output internal/perfbench/reports/baseline.json
 
 help:
-	@echo "Targets: build (default), build-all, test, test-quick, vet, fmt, fmt-check, lint, tidy, clean, baseline"
+	@echo "Targets: build (default), build-all, test, test-quick, vet, fmt, fmt-check, lint, lint-static, deadcode, vulncheck, tidy, clean, baseline"

README.md

@@ -131,6 +131,7 @@ export ANTHROPIC_API_KEY=...
 export GEMINI_API_KEY=...
 export AIMLAPI_API_KEY=...
 export LONGCAT_API_KEY=...
+export FIREWORKS_API_KEY=...
 export MINIMAX_API_KEY=...
 export MINIMAXI_API_KEY=...
 ```
@@ -147,6 +148,12 @@ To configure Meituan LongCat (LongCat-2.0) directly, run:
 zero providers setup longcat --set-active
 ```
 
+To configure Fireworks AI directly, run:
+
+```bash
+zero providers setup fireworks --set-active
+```
+
 MiniMax presets use the Anthropic-compatible endpoints for the global and China
 regions:
 
@@ -367,22 +374,16 @@ variable unset and set.
 
 ### Code Quality and Security Checks
 
-Before committing any changes, ensure all Go code quality and security checks pass. Pinned `go run` commands matching CI constraints can be used directly without prior installation:
+Before committing any changes, ensure all Go code quality and security checks pass. The `make` targets below pin each tool to this module's Go version, so they load correctly even when your default `go` toolchain is older — running the plain `go run ...@version` form yourself can select the tool module's own (older) toolchain and fail to load this module instead.
 
-1. **Formatting**: Run `go fmt ./...` (or `make fmt`).
-2. **Vetting**: Run `go vet ./...` (or `make vet`).
-3. **Linting**: Run `go run github.com/golangci/golangci-lint/v2/cmd/[email protected] run --enable-only unused,ineffassign,staticcheck ./...`.
-4. **Vulnerability Scan**: Run `go run golang.org/x/vuln/cmd/[email protected] ./...`.
+1. **Formatting**: Run `make fmt` (or `go fmt ./...`).
+2. **Vetting**: Run `make vet` (or `go vet ./...`).
+3. **Linting**: Run `make lint-static`.
+4. **Vulnerability Scan**: Run `make vulncheck`.
 
-If you prefer to install these tools globally on your path, you can run:
-
-```bash
-# Install golangci-lint
-go install github.com/golangci/golangci-lint/v2/cmd/[email protected]
-
-# Install govulncheck
-go install golang.org/x/vuln/cmd/[email protected]
-```
+Use the repository-managed targets rather than globally installed binaries:
+the targets apply the module's required Go toolchain as well as the reviewed,
+pinned tool versions.
 
 The installed binaries land in `$GOBIN` when it is set, otherwise in
 `$GOPATH/bin` (default `~/go/bin`). That directory must be on your `PATH` to

docs/EXTENDING.md

@@ -108,10 +108,10 @@ Discovery roots (earlier wins on name collisions):
 2. **Shared multi-agent dir** — `~/.agents/skills/` when present (read-only discovery; never an install target)
 3. **Plugin skill roots** — skills bundled by active plugins (section 6)
 
-A missing directory is fine — Zero just omits it. Management commands (`zero skills add` / `remove` / `lock`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`.
+A missing directory is fine — Zero just omits it. Management commands (`zero skill add` / `zero skill remove`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`. `skill info` reports lock source/hash when present and whether the on-disk `SKILL.md` has drifted from the recorded hash.
 
 ```text
-~/.local/share/zero/skills/          # primary (install/remove/lock target)
+~/.local/share/zero/skills/          # primary (install/remove target)
   run-benchmarks/
     SKILL.md
   write-changelog/

docs/HOW_ZERO_WORKS.md

@@ -749,7 +749,7 @@ flowchart TD
 - **Plugins** can add tools, hooks, and skill roots. Bootstrap always registers a
   multi-root skill tool: primary Zero skills dir, optional `~/.agents/skills`,
   then plugin skill roots (earlier wins). `internal/skills` owns that merge via
-  `LoadFromRoots` / `DiscoveryRoots`; single-root `Load` remains for install/write.
+  `LoadFromRoots`; single-root `Load` remains for install/write.
 - **Hooks** can observe or block tool lifecycle events.
 
 ## End-to-End Data Flow

internal/acp/agent.go

@@ -90,9 +90,6 @@ func NewAgent(conn *Conn, deps Deps) *Agent {
 	return a
 }
 
-// Serve runs the connection read loop until the stream closes or ctx is done.
-func (a *Agent) Serve(ctx context.Context) error { return a.conn.Serve(ctx) }
-
 // ---- initialize ----
 
 func (a *Agent) handleInitialize(_ context.Context, params json.RawMessage) (any, error) {

internal/acp/export_test.go

@@ -0,0 +1,11 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package acp
+
+import "context"
+
+// Serve runs the connection read loop until the stream closes or ctx is done.
+func (a *Agent) Serve(ctx context.Context) error { return a.conn.Serve(ctx) }
+
+func ImageBlock(base64Data, mimeType string) ContentBlock {
+	return ContentBlock{Type: "image", Data: base64Data, MimeType: mimeType}
+}

internal/acp/jsonrpc.go

@@ -17,6 +17,7 @@ import (
 	"fmt"
 	"io"
 	"sync"
+	"sync/atomic"
 )
 
 // JSON-RPC 2.0 standard error codes.
@@ -80,8 +81,9 @@ type NotifyFunc func(ctx context.Context, params json.RawMessage)
 // requests/notifications — needed because ACP is bidirectional (the agent calls
 // the client for session/request_permission, fs/*, terminal/*).
 type Conn struct {
-	reader *bufio.Reader
-	w      io.Writer
+	rawReader    io.Reader // wrapped lazily in Serve, once ctx is known — see interruptibleReader
+	readerCloser io.Closer
+	w            io.Writer
 
 	writeMu sync.Mutex // serializes all writes to w
 
@@ -96,10 +98,15 @@ type Conn struct {
 	wg sync.WaitGroup // tracks in-flight inbound handlers
 }
 
-// NewConn builds a peer reading ndjson from r and writing ndjson to w.
+// NewConn builds a peer reading ndjson from r and writing ndjson to w. This
+// Conn does not own r: cancelling Serve's context cannot forcibly close it, so
+// a Read on r that's genuinely idle (never returns on its own) leaves Serve
+// blocked until r itself produces something — e.g. a test that closes r's own
+// write side to unblock it. Use NewOwnedConn when this Conn should own r's
+// lifetime.
 func NewConn(r io.Reader, w io.Writer) *Conn {
 	return &Conn{
-		reader:    bufio.NewReader(r),
+		rawReader: r,
 		w:         w,
 		handlers:  make(map[string]HandlerFunc),
 		notifiers: make(map[string]NotifyFunc),
@@ -107,6 +114,18 @@ func NewConn(r io.Reader, w io.Writer) *Conn {
 	}
 }
 
+// NewOwnedConn is like NewConn but declares that this Conn exclusively owns r:
+// if r implements io.Closer, Serve closes it on context cancellation to
+// interrupt an otherwise-idle blocking Read. Only pass a reader whose lifetime
+// this Conn should control — e.g. the process's own stdin — never a reader a
+// caller retains or shares elsewhere, since cancellation tears it down
+// permanently for every consumer, not just this Conn.
+func NewOwnedConn(r io.Reader, w io.Writer) *Conn {
+	c := NewConn(r, w)
+	c.readerCloser, _ = r.(io.Closer)
+	return c
+}
+
 // Handle registers a request handler for method.
 func (c *Conn) Handle(method string, fn HandlerFunc) { c.handlers[method] = fn }
 
@@ -128,17 +147,36 @@ func (c *Conn) Serve(ctx context.Context) error {
 		c.wg.Wait()
 	}()
 
+	interruptible := newInterruptibleReader(ctx, c.rawReader, c.readerCloser)
+	reader := bufio.NewReader(interruptible)
+
 	for {
 		// One ndjson value per line. Framing per line (rather than streaming the
 		// decoder) keeps a single malformed line from making the whole connection
 		// unrecoverable — we report -32700 and continue.
-		line, err := c.reader.ReadBytes('\n')
+		//
+		// generation brackets this SPECIFIC ReadBytes call: bufio may invoke
+		// interruptible.Read zero or more times underneath it (zero when the
+		// answer is already sitting in bufio's own buffer — including a
+		// previously-buffered, not-yet-surfaced error: bufio can return a
+		// complete line with a nil error while quietly caching an error it
+		// received ALONGSIDE that line's bytes, which then surfaces on the VERY
+		// NEXT call with no further contact with interruptible.Read at all). The
+		// generation only advances inside interruptible.Read's own cancellation
+		// branch, so "unchanged across this call" is proof this call's outcome
+		// didn't pass through that branch — whether because it was answered from
+		// the buffer or because it raced ctx.Done() and won. Only that proof
+		// makes it safe to call the outcome genuine.
+		before := interruptible.generation()
+		line, err := reader.ReadBytes('\n')
+		interrupted := interruptible.generation() != before
+
 		if len(bytes.TrimSpace(line)) > 0 {
 			c.handleLine(ctx, line)
 		}
 		if err != nil {
 			c.failAllPending(err)
-			if errors.Is(err, io.EOF) || ctx.Err() != nil {
+			if errors.Is(err, io.EOF) || interrupted {
 				return nil
 			}
 			return err
@@ -146,6 +184,100 @@ func (c *Conn) Serve(ctx context.Context) error {
 	}
 }
 
+// interruptibleReader wraps a reader so a context cancellation can unblock a
+// currently in-flight Read without ever discarding — or misattributing — the
+// real outcome of a call that wasn't actually blocked.
+//
+// Each Read runs the underlying call on its own goroutine and races it against
+// ctx.Done(). If the underlying call finishes first, its result is returned
+// exactly as received; ctx being cancelled a moment later has no bearing on a
+// call that already had its answer. If ctx.Done() fires first, closer (when
+// non-nil) is closed to nudge a genuinely blocked read, the generation counter
+// is bumped, and the call STILL waits for and returns the underlying call's own
+// result — never a synthesized one — since closing a reader's outcome is
+// whatever the reader itself reports for it, not ours to invent.
+//
+// The counter is what lets Serve tell a call that was actually raced against
+// cancellation apart from one merely answered while cancellation happened to be
+// pending — the distinction the read loop's own doc comment explains.
+type interruptibleReader struct {
+	inner  io.Reader
+	closer io.Closer
+	ctx    context.Context
+
+	closeOnce sync.Once
+	gen       atomic.Int64
+}
+
+func newInterruptibleReader(ctx context.Context, inner io.Reader, closer io.Closer) *interruptibleReader {
+	return &interruptibleReader{inner: inner, closer: closer, ctx: ctx}
+}
+
+func (r *interruptibleReader) generation() int64 { return r.gen.Load() }
+
+type interruptibleReadResult struct {
+	n   int
+	err error
+}
+
+// interruptibleReadDecision arbitrates whether an underlying Read completed or
+// cancellation claimed it first. Completion is recorded before the result is
+// published so cancellation cannot misclassify a finished read merely because
+// its goroutine has not sent the result yet.
+type interruptibleReadDecision struct {
+	once sync.Once
+}
+
+func (d *interruptibleReadDecision) complete() { d.once.Do(func() {}) }
+
+func (d *interruptibleReadDecision) interrupt() (claimed bool) {
+	d.once.Do(func() { claimed = true })
+	return claimed
+}
+
+// readInner records completion in the Read call's deferred epilogue. It also
+// converts a reader panic into a terminal error so the helper goroutine always
+// publishes a result for Read to join.
+func (r *interruptibleReader) readInner(p []byte, decision *interruptibleReadDecision) (res interruptibleReadResult) {
+	defer func() {
+		if recovered := recover(); recovered != nil {
+			res = interruptibleReadResult{err: fmt.Errorf("acp: reader panicked: %v", recovered)}
+		}
+		decision.complete()
+	}()
+	res.n, res.err = r.inner.Read(p)
+	return res
+}
+
+func (r *interruptibleReader) Read(p []byte) (int, error) {
+	resultCh := make(chan interruptibleReadResult, 1)
+	var decision interruptibleReadDecision
+	go func() {
+		resultCh <- r.readInner(p, &decision)
+	}()
+	select {
+	case res := <-resultCh:
+		return res.n, res.err
+	case <-r.ctx.Done():
+		// resultCh may not be readable yet even when inner.Read has returned: its
+		// goroutine can be descheduled between recording completion and publishing
+		// the result. Only interrupt a call that cancellation actually claims.
+		if decision.interrupt() {
+			r.gen.Add(1)
+			if r.closer != nil {
+				r.closeOnce.Do(func() { _ = r.closer.Close() })
+			}
+		}
+		// Wait for the real result rather than returning synthetically: p is
+		// still being written by the goroutine above until it does, so returning
+		// early would race that write, and the actual error (whatever closing
+		// the reader produced, or whatever the reader was already about to
+		// report) is what the caller should see.
+		res := <-resultCh
+		return res.n, res.err
+	}
+}
+
 // handleLine decodes and dispatches one ndjson frame. A parse failure replies
 // with -32700 (id null) and keeps the connection alive.
 func (c *Conn) handleLine(ctx context.Context, line []byte) {

internal/acp/jsonrpc_test.go

@@ -3,11 +3,30 @@ package acp
 import (
 	"context"
 	"encoding/json"
+	"errors"
 	"io"
+	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 )
 
+type testReadCloser struct {
+	read  func([]byte) (int, error)
+	close func() error
+}
+
+func (r testReadCloser) Read(p []byte) (int, error) { return r.read(p) }
+func (r testReadCloser) Close() error               { return r.close() }
+
+type testReader func([]byte) (int, error)
+
+func (r testReader) Read(p []byte) (int, error) { return r(p) }
+
+type testWriter func([]byte) (int, error)
+
+func (w testWriter) Write(p []byte) (int, error) { return w(p) }
+
 // connPair wires two Conns together over in-memory pipes and serves both.
 func connPair(t *testing.T) (a, b *Conn, stop func()) {
 	t.Helper()
@@ -25,6 +44,275 @@ func connPair(t *testing.T) (a, b *Conn, stop func()) {
 	}
 }
 
+func TestConnServeCancellationInterruptsIdleRead(t *testing.T) {
+	pipeReader, writer := io.Pipe()
+	t.Cleanup(func() { _ = writer.Close() })
+	// Buffered + non-blocking send, not close: interruptibleReader retries each
+	// underlying Read on its own goroutine, so if bufio ever calls this more than
+	// once, a close() here would panic on the second call.
+	readStarted := make(chan struct{}, 1)
+	closeCalls := make(chan struct{}, 2)
+	reader := testReadCloser{
+		read: func(p []byte) (int, error) {
+			select {
+			case readStarted <- struct{}{}:
+			default:
+			}
+			return pipeReader.Read(p)
+		},
+		close: func() error {
+			closeCalls <- struct{}{}
+			return pipeReader.Close()
+		},
+	}
+	conn := NewOwnedConn(reader, io.Discard)
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan error, 1)
+	go func() { done <- conn.Serve(ctx) }()
+
+	<-readStarted
+	cancel()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("Serve returned %v after cancellation, want nil", err)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("Serve did not return after cancelling an idle connection")
+	}
+	if got := len(closeCalls); got != 1 {
+		t.Fatalf("reader Close calls = %d, want 1", got)
+	}
+}
+
+func TestConnServePreservesTerminalReadErrorDuringCancellation(t *testing.T) {
+	for _, closable := range []bool{false, true} {
+		name := "non-closable"
+		if closable {
+			name = "closable"
+		}
+		t.Run(name, func(t *testing.T) {
+			wantErr := errors.New("read failed")
+			read := testReader(func(p []byte) (int, error) {
+				return copy(p, "not json"), wantErr
+			})
+			closeCalls := make(chan struct{}, 1)
+			var reader io.Reader = read
+			if closable {
+				reader = testReadCloser{
+					read: read,
+					close: func() error {
+						closeCalls <- struct{}{}
+						return nil
+					},
+				}
+			}
+			// Buffered + non-blocking send, not close: a later change adding a
+			// second write (e.g. an additional error frame) must not panic on a
+			// repeated close of an already-closed channel.
+			writeStarted := make(chan struct{}, 1)
+			releaseWrite := make(chan struct{})
+			writer := testWriter(func(p []byte) (int, error) {
+				select {
+				case writeStarted <- struct{}{}:
+				default:
+				}
+				<-releaseWrite
+				return len(p), nil
+			})
+			newConn := NewConn
+			if closable {
+				// Ownership matters here: this case exists to prove that even a
+				// Conn entitled to close its reader on cancellation still
+				// preserves a terminal error that was already decided.
+				newConn = NewOwnedConn
+			}
+			conn := newConn(reader, writer)
+			ctx, cancel := context.WithCancel(context.Background())
+			done := make(chan error, 1)
+			go func() { done <- conn.Serve(ctx) }()
+
+			<-writeStarted
+			cancel()
+			close(releaseWrite)
+			select {
+			case err := <-done:
+				if !errors.Is(err, wantErr) {
+					t.Fatalf("Serve returned %v, want terminal read error %v", err, wantErr)
+				}
+			case <-time.After(time.Second):
+				t.Fatal("Serve did not return after terminal read error")
+			}
+			if got := len(closeCalls); got != 0 {
+				t.Fatalf("reader Close calls = %d, want 0 after terminal read", got)
+			}
+		})
+	}
+}
+
+// TestConnServePreservesReadErrorBufferedAlongsideAPriorLine is the deterministic
+// regression test for jatmn's #782 finding: a real, non-EOF ReadBytes failure can
+// be DECIDED before cancellation ever happens, yet only SURFACE afterward, and
+// must still be reported rather than swallowed as a clean shutdown.
+//
+// bufio.Reader can obtain data AND a terminal error from ONE underlying Read call
+// (io.Reader explicitly permits returning (n>0, err) together). If a delimiter is
+// found within that data, ReadBytes returns the line with a nil error for THIS
+// call, but bufio caches the error internally and returns it — WITHOUT calling the
+// underlying reader again — on the very next ReadBytes call. This test forces that
+// exact sequence: the underlying reader hands back one complete, validly-framed
+// line together with wantErr in a SINGLE call (asserted below to be its ONLY
+// call), the loop dispatches that line through a synchronous path that blocks the
+// read loop, cancellation is asserted to have happened before the loop attempts
+// its next read, and only then is the block released. By the time the read loop
+// asks for more input, cancellation is already in effect and the underlying
+// reader is never touched again — so if Serve attributed the resulting error to
+// cancellation merely because ctx was already done, it would incorrectly return
+// nil. It must instead recognize that this read was answered entirely from
+// bufio's own cache, never raced against ctx.Done(), and report wantErr.
+func TestConnServePreservesReadErrorBufferedAlongsideAPriorLine(t *testing.T) {
+	wantErr := errors.New("read failed")
+	var readCalls atomic.Int32
+	read := testReader(func(p []byte) (int, error) {
+		readCalls.Add(1)
+		// An unsupported jsonrpc version with an id takes the SYNCHRONOUS
+		// writeError path in handleLine (not a dispatched goroutine), giving this
+		// test a reliable blocking point between processing this line and the
+		// loop's next ReadBytes call.
+		return copy(p, `{"jsonrpc":"1.0","id":1}`+"\n"), wantErr
+	})
+	writeStarted := make(chan struct{}, 1)
+	releaseWrite := make(chan struct{})
+	var releaseWriteOnce sync.Once
+	release := func() { releaseWriteOnce.Do(func() { close(releaseWrite) }) }
+	t.Cleanup(release)
+	writer := testWriter(func(p []byte) (int, error) {
+		select {
+		case writeStarted <- struct{}{}:
+		default:
+		}
+		<-releaseWrite
+		return len(p), nil
+	})
+	conn := NewConn(read, writer)
+	ctx, cancel := context.WithCancel(context.Background())
+	done := make(chan error, 1)
+	go func() { done <- conn.Serve(ctx) }()
+
+	// The synchronous write confirms the first (and, per the panic guard above,
+	// only) read has already returned and is being processed — cancelling now
+	// lands squarely in the gap between that read and the loop's next one, never
+	// racing an in-flight interruptibleReader.Read call.
+	select {
+	case <-writeStarted:
+	case <-time.After(time.Second):
+		t.Fatal("Serve did not reach the synchronous write")
+	}
+	cancel()
+	release()
+
+	select {
+	case err := <-done:
+		if !errors.Is(err, wantErr) {
+			t.Fatalf("Serve returned %v, want the buffered terminal read error %v", err, wantErr)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("Serve did not return after the buffered terminal read error")
+	}
+	if got := readCalls.Load(); got != 1 {
+		t.Fatalf("underlying Read calls = %d, want exactly 1 (bufio must have served the second ReadBytes from its cache)", got)
+	}
+}
+
+// TestInterruptibleReaderPrefersDecidedResultOverCancellation is the regression
+// test for jatmn's second #782 finding: interruptibleReader.Read's select
+// between resultCh and ctx.Done() chooses pseudo-randomly when both are ready,
+// so a real, already-decided outcome (e.g. a terminal transport error) could be
+// misattributed to cancellation purely because ctx also happened to be done by
+// the time the select ran. Each iteration lets the underlying Read return (so
+// resultCh already holds the real outcome) before cancelling, maximizing the
+// odds of landing exactly in that ambiguous window; the generation counter must
+// never advance when the real, already-decided result is what gets returned.
+func TestInterruptibleReaderPrefersDecidedResultOverCancellation(t *testing.T) {
+	wantErr := errors.New("read failed")
+	for i := 0; i < 100; i++ {
+		resultReady := make(chan struct{})
+		read := testReader(func(p []byte) (int, error) {
+			defer close(resultReady)
+			return copy(p, "x"), wantErr
+		})
+		ctx, cancel := context.WithCancel(context.Background())
+		r := newInterruptibleReader(ctx, read, nil)
+
+		readDone := make(chan interruptibleReadResult, 1)
+		go func() {
+			n, err := r.Read(make([]byte, 8))
+			readDone <- interruptibleReadResult{n, err}
+		}()
+
+		<-resultReady
+		time.Sleep(time.Millisecond) // bias resultCh's send ahead of cancel
+		cancel()
+
+		res := <-readDone
+		if !errors.Is(res.err, wantErr) {
+			t.Fatalf("iteration %d: Read returned %v, want %v", i, res.err, wantErr)
+		}
+		if got := r.generation(); got != 0 {
+			t.Fatalf("iteration %d: generation = %d, want 0 (a decided result must never be attributed to cancellation)", i, got)
+		}
+		cancel()
+	}
+}
+
+// TestInterruptibleReadCompletionWinsBeforeResultPublication exercises the
+// actual inner-read wrapper at the handoff between the underlying Read and the
+// helper goroutine's result publication. Cancellation must not reclassify a
+// result returned by this wrapper as interrupted.
+func TestInterruptibleReadCompletionWinsBeforeResultPublication(t *testing.T) {
+	wantErr := errors.New("read failed")
+	read := testReader(func(p []byte) (int, error) {
+		return copy(p, "x"), wantErr
+	})
+	r := newInterruptibleReader(context.Background(), read, nil)
+	var decision interruptibleReadDecision
+	readComplete := make(chan struct{})
+	resultCh := make(chan interruptibleReadResult)
+	go func() {
+		res := r.readInner(make([]byte, 8), &decision)
+		close(readComplete)
+		resultCh <- res
+	}()
+
+	<-readComplete
+	if decision.interrupt() {
+		t.Fatal("cancellation claimed a read that completed before result publication")
+	}
+	res := <-resultCh
+	if res.n != 1 || !errors.Is(res.err, wantErr) {
+		t.Fatalf("inner read result = (%d, %v), want (1, %v)", res.n, res.err, wantErr)
+	}
+}
+
+func TestConnServeReportsReaderPanicAsTerminalError(t *testing.T) {
+	read := testReader(func([]byte) (int, error) {
+		panic("boom")
+	})
+	conn := NewConn(read, io.Discard)
+	done := make(chan error, 1)
+	go func() { done <- conn.Serve(context.Background()) }()
+
+	select {
+	case err := <-done:
+		const want = "acp: reader panicked: boom"
+		if err == nil || err.Error() != want {
+			t.Fatalf("Serve returned %v, want %q", err, want)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("Serve did not return after reader panic")
+	}
+}
+
 func TestConnRequestResponse(t *testing.T) {
 	a, b, stop := connPair(t)
 	defer stop()

internal/acp/types.go

@@ -103,10 +103,6 @@ type ContentBlock struct {
 
 func TextBlock(text string) ContentBlock { return ContentBlock{Type: "text", Text: text} }
 
-func ImageBlock(base64Data, mimeType string) ContentBlock {
-	return ContentBlock{Type: "image", Data: base64Data, MimeType: mimeType}
-}
-
 // ---- sessions ----
 
 // McpServer mirrors the editor-provided MCP server entry. ZERO owns its own MCP
@@ -233,10 +229,6 @@ func ToolContent(block ContentBlock) ToolCallContent {
 	return ToolCallContent{Type: "content", Content: &block}
 }
 
-func ToolDiff(path, oldText, newText string) ToolCallContent {
-	return ToolCallContent{Type: "diff", Path: path, OldText: oldText, NewText: newText}
-}
-
 type ToolCallLocation struct {
 	Path string `json:"path"`
 	Line *int   `json:"line,omitempty"`

internal/agent/additional_permissions_validation_test.go

@@ -5,9 +5,29 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/Gitlawb/zero/internal/sandbox"
 	"github.com/Gitlawb/zero/internal/tools"
 )
 
+type normalizationRecordingShellTool struct {
+	calls []map[string]any
+}
+
+func (tool *normalizationRecordingShellTool) Name() string { return "bash" }
+func (tool *normalizationRecordingShellTool) Description() string {
+	return "record normalized shell arguments"
+}
+func (tool *normalizationRecordingShellTool) Parameters() tools.Schema {
+	return tools.Schema{Type: "object", AdditionalProperties: true}
+}
+func (tool *normalizationRecordingShellTool) Safety() tools.Safety {
+	return tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionAllow}
+}
+func (tool *normalizationRecordingShellTool) Run(_ context.Context, args map[string]any) tools.Result {
+	tool.calls = append(tool.calls, cloneArgs(args))
+	return tools.Result{Status: tools.StatusOK}
+}
+
 // A shell call that sets sandbox_permissions: with_additional_permissions but
 // omits (or malforms) additional_permissions can never succeed: the same
 // validation runs again at grant time regardless of the user's decision. It
@@ -16,7 +36,7 @@ import (
 func TestExecuteToolCallRejectsMissingAdditionalPermissionsBeforePrompt(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 
 	promptCalls := 0
 	result, err := executeToolCall(
@@ -51,7 +71,7 @@ func TestExecuteToolCallRejectsMissingAdditionalPermissionsBeforePrompt(t *testi
 func TestExecuteToolCallRejectsAdditionalPermissionsWithoutFlagBeforePrompt(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 
 	promptCalls := 0
 	result, err := executeToolCall(
@@ -88,7 +108,7 @@ func TestExecuteToolCallRejectsAdditionalPermissionsWithoutFlagBeforePrompt(t *t
 func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 
 	result, err := executeToolCall(
 		context.Background(),
@@ -107,9 +127,7 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
 	if !strings.Contains(result.Output, `{"network": {"enabled": true}}`) {
 		t.Fatalf("output = %q, want a concrete additional_permissions example", 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)
-	}
+	assertNamesOmitRemedy(t, "missing additional_permissions", result.Output)
 }
 
 // A well-formed additional_permissions payload must still reach the normal
@@ -117,7 +135,7 @@ func TestExecuteToolCallMissingAdditionalPermissionsErrorIsActionable(t *testing
 func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 
 	promptCalls := 0
 	_, err := executeToolCall(
@@ -144,3 +162,237 @@ func TestExecuteToolCallStillPromptsForValidAdditionalPermissions(t *testing.T)
 		t.Fatalf("OnPermissionRequest called %d times, want exactly 1 for a valid elevation request", promptCalls)
 	}
 }
+
+// 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),
+		string(tools.SandboxPermissionsWithAdditionalPermissions),
+	} {
+		args := map[string]any{
+			"command":             "./zero --version",
+			"sandbox_permissions": mode,
+			"additional_permissions": map[string]any{
+				"file_system": map[string]any{
+					"deny_read": []any{},
+					"entries":   []any{},
+					"read":      []any{},
+					"write":     []any{},
+				},
+				"network": map[string]any{"enabled": false},
+			},
+		}
+		normalizeNoopAdditionalPermissions(args, t.TempDir(), nil)
+		if _, exists := args["additional_permissions"]; exists {
+			t.Fatalf("mode %q retained empty additional_permissions: %#v", mode, args)
+		}
+		if got := args["sandbox_permissions"]; got != string(tools.SandboxPermissionsUseDefault) {
+			t.Fatalf("mode %q normalized to %q, want use_default", mode, got)
+		}
+	}
+}
+
+func TestExecuteToolCallNormalizesProviderExpandedEmptyAdditionalPermissions(t *testing.T) {
+	tool := &normalizationRecordingShellTool{}
+	registry := tools.NewRegistry()
+	registry.Register(tool)
+
+	result, err := executeToolCall(
+		context.Background(),
+		registry,
+		ToolCall{
+			ID:   "c1",
+			Name: "bash",
+			Arguments: `{"command":"./zero --version","sandbox_permissions":"with_additional_permissions",` +
+				`"additional_permissions":{"file_system":{"deny_read":[],"entries":[],"read":[],"write":[]},"network":{"enabled":false}}}`,
+		},
+		PermissionModeAuto,
+		Options{Cwd: t.TempDir()},
+	)
+	if err != nil {
+		t.Fatalf("executeToolCall: %v", err)
+	}
+	if result.Status != tools.StatusOK {
+		t.Fatalf("provider-expanded empty permissions were rejected: %s", result.Output)
+	}
+	if len(tool.calls) != 1 {
+		t.Fatalf("shell tool calls = %d, want 1", len(tool.calls))
+	}
+	args := tool.calls[0]
+	if _, exists := args["additional_permissions"]; exists {
+		t.Fatalf("executeToolCall retained empty additional_permissions: %#v", args)
+	}
+	if got := args["sandbox_permissions"]; got != string(tools.SandboxPermissionsUseDefault) {
+		t.Fatalf("sandbox mode = %q, want use_default", got)
+	}
+}
+
+func TestNormalizeAdditionalPermissionsPreservesRealRequest(t *testing.T) {
+	args := map[string]any{
+		"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
+		"additional_permissions": map[string]any{
+			"network": map[string]any{"enabled": true},
+		},
+	}
+	normalizeNoopAdditionalPermissions(args, t.TempDir(), nil)
+	if _, exists := args["additional_permissions"]; !exists {
+		t.Fatal("non-empty additional_permissions was removed")
+	}
+	if got := args["sandbox_permissions"]; got != string(tools.SandboxPermissionsWithAdditionalPermissions) {
+		t.Fatalf("sandbox mode changed to %q", got)
+	}
+}
+
+func TestNormalizeAdditionalPermissionsRemovesWorkspaceReadAlreadyCoveredBySandbox(t *testing.T) {
+	root := t.TempDir()
+	engine := sandbox.NewEngine(sandbox.EngineOptions{
+		WorkspaceRoot: root,
+		Policy:        sandbox.DefaultPolicy(),
+	})
+	args := map[string]any{
+		"command":             "./zero --version",
+		"sandbox_permissions": string(tools.SandboxPermissionsWithAdditionalPermissions),
+		"additional_permissions": map[string]any{
+			"file_system": map[string]any{
+				"entries": []any{
+					map[string]any{
+						"access": "read",
+						"path": map[string]any{
+							"type": "path",
+							"path": ".",
+						},
+					},
+				},
+				"deny_read": []any{},
+				"read":      []any{},
+				"write":     []any{},
+			},
+			"network": map[string]any{"enabled": false},
+		},
+	}
+
+	normalizeNoopAdditionalPermissions(args, root, engine)
+
+	if _, exists := args["additional_permissions"]; exists {
+		t.Fatalf("already-covered additional_permissions was retained: %#v", args)
+	}
+	if got := args["sandbox_permissions"]; got != string(tools.SandboxPermissionsUseDefault) {
+		t.Fatalf("sandbox mode changed to %q", got)
+	}
+}

internal/agent/command_prefix.go

@@ -501,15 +501,3 @@ func hasStringPrefix(values []string, prefix []string) bool {
 	}
 	return true
 }
-
-func equalStringSlices(left []string, right []string) bool {
-	if len(left) != len(right) {
-		return false
-	}
-	for index := range left {
-		if left[index] != right[index] {
-			return false
-		}
-	}
-	return true
-}

internal/agent/compaction.go

@@ -413,6 +413,14 @@ func (state *compactionState) maybeCompact(
 	// the bodies of old, large tool results (the model has already acted on
 	// them). If that brings us back under threshold, skip the paid summarizer
 	// entirely and preserve recent turns verbatim.
+	if pruned, reclaimed := pruneSupersededReadResults(messages); reclaimed > 0 {
+		messages = pruned
+		size = state.calibratedTokens(estimateTokens(messages) + toolTokens)
+		if size <= state.threshold {
+			state.lowWaterMark = size
+			return messages
+		}
+	}
 	if pruned, reclaimed := pruneStaleToolOutput(messages, state.preserveLast); reclaimed > 0 {
 		messages = pruned
 		size = state.calibratedTokens(estimateTokens(messages) + toolTokens)

internal/agent/compaction_preserve.go

@@ -610,15 +610,6 @@ func formatPreservedState(plan string, task *preservedTaskState, edits, tools, s
 	return preservedStateLabel + "\n" + string(encoded)
 }
 
-// parsePreservedState recovers the plan + skills from a prior summary's preserved
-// block. JSON escaping makes this lossless even when a skill body contains
-// markdown headings, code fences, or quotes. Returns ("", nil) when absent or
-// malformed.
-func parsePreservedState(summaryContent string) (string, []skillEntry) {
-	state := parsePreservedStateBlock(summaryContent)
-	return state.Plan, preservedSkillsToEntries(state.Skills)
-}
-
 func parsePreservedStateBlock(summaryContent string) preservedState {
 	idx := strings.LastIndex(summaryContent, preservedStateLabel)
 	if idx < 0 {

internal/agent/compaction_test.go

@@ -336,7 +336,7 @@ func TestRunProactiveCompactionTriggers(t *testing.T) {
 	}
 
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(t.TempDir()))
+	registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil))
 
 	result, err := Run(context.Background(), strings.Repeat("y", 8000), provider, Options{
 		Registry:               registry,
@@ -364,7 +364,7 @@ func TestRunNoCompactionWhenContextWindowZero(t *testing.T) {
 		},
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(t.TempDir()))
+	registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil))
 
 	_, err := Run(context.Background(), strings.Repeat("y", 8000), provider, Options{
 		Registry:       registry,
@@ -379,6 +379,43 @@ func TestRunNoCompactionWhenContextWindowZero(t *testing.T) {
 	}
 }
 
+func TestMaybeCompactStopsAfterSupersededReadsReachThreshold(t *testing.T) {
+	body := strings.Repeat("same source line\n", 1000)
+	messages := []zeroruntime.Message{
+		{Role: zeroruntime.MessageRoleSystem, Content: "system"},
+		toolCallWithArgs("old", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "old", Content: body},
+		toolCallWithArgs("new", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "new", Content: body},
+	}
+	pruned, reclaimed := pruneSupersededReadResults(messages)
+	if reclaimed <= 0 {
+		t.Fatal("test setup did not reclaim superseded read output")
+	}
+	prunedSize := estimateTokens(pruned)
+	if estimateTokens(messages) <= prunedSize {
+		t.Fatal("test setup does not cross the compaction threshold")
+	}
+
+	provider := &summarizeRecordingProvider{}
+	state := &compactionState{
+		enabled:      true,
+		threshold:    prunedSize,
+		preserveLast: 2,
+	}
+	got := state.maybeCompact(context.Background(), provider, messages, nil)
+
+	if provider.summarizeCalls != 0 {
+		t.Fatalf("cheap pruning should avoid the summarizer, got %d calls", provider.summarizeCalls)
+	}
+	if state.lowWaterMark != prunedSize {
+		t.Fatalf("lowWaterMark = %d, want %d", state.lowWaterMark, prunedSize)
+	}
+	if !strings.Contains(got[2].Content, "superseded identical") || got[4].Content != body {
+		t.Fatalf("unexpected pruned messages: %#v", got)
+	}
+}
+
 // reactiveProvider builds up history with a tool call on turn 1, then errors
 // with a context-limit message on turn 2 (once the history is large enough that
 // compaction can shrink it), then succeeds on the same-turn retry. Summary
@@ -427,7 +464,7 @@ func TestRunReactiveCompactionRecovers(t *testing.T) {
 	// the compaction closure. read_file also returns a sizeable result that
 	// bloats the history.
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(t.TempDir()))
+	registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil))
 
 	// ContextWindow large enough that proactive compaction never triggers, so
 	// only the reactive path can save the run.
@@ -491,7 +528,7 @@ func TestRunReactiveRetryDoesNotDoubleEmitText(t *testing.T) {
 		finalText:   "recovered",
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(t.TempDir()))
+	registry.Register(tools.NewScopedReadFileTool(t.TempDir(), nil))
 
 	var deltas []string
 	result, err := Run(context.Background(), strings.Repeat("z", 6000), provider, Options{

internal/agent/deferred_loop_test.go

@@ -150,7 +150,7 @@ func assertNoDeferredDiscoveryMessage(t *testing.T, request zeroruntime.Completi
 func TestPartitionToolsInactiveIsByteIdenticalAndDropsToolSearch(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	registry.Register(fakeDeferredTool{name: "mcp__srv__a", desc: "tool a"})
 	registry.Register(fakeToolSearchTool{})
 
@@ -305,8 +305,8 @@ func TestPartitionToolsActiveExcludesDisabledDeferredFromDiscoveryAndExposed(t *
 func TestPartitionToolsActiveHidesUnloadedExposesLoaded(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root)) // non-deferred builtin
-	registry.Register(fakeToolSearchTool{})        // non-deferred, must stay exposed
+	registry.Register(tools.NewScopedReadFileTool(root, nil)) // non-deferred builtin
+	registry.Register(fakeToolSearchTool{})                   // non-deferred, must stay exposed
 	registry.Register(fakeDeferredTool{name: "mcp__srv__alpha", desc: "alpha tool"})
 	registry.Register(fakeDeferredTool{name: "mcp__srv__beta", desc: "beta tool"})
 
@@ -341,15 +341,15 @@ func TestPartitionToolsActiveHidesUnloadedExposesLoaded(t *testing.T) {
 	if search.Description != discovery || !strings.Contains(search.Description, "mcp") {
 		t.Fatalf("tool_search description must carry discovery source, got %q", search.Description)
 	}
-	if strings.Contains(search.Description, "mcp__srv__alpha") {
-		t.Fatalf("tool_search description must not list already-loaded tool names, got %q", search.Description)
+	if !strings.Contains(search.Description, "mcp__srv__alpha") {
+		t.Fatalf("stable tool_search catalog must retain already-loaded tool names, got %q", search.Description)
 	}
 	if !strings.Contains(search.Description, "mcp__srv__beta") {
 		t.Fatalf("tool_search description must list exact hidden tool names, got %q", search.Description)
 	}
 }
 
-func TestPartitionToolsActiveNothingHiddenEmptyReminder(t *testing.T) {
+func TestPartitionToolsActiveNothingHiddenKeepsStableCatalog(t *testing.T) {
 	registry := tools.NewRegistry()
 	registry.Register(fakeDeferredTool{name: "mcp__srv__alpha", desc: "alpha"})
 	registry.Register(fakeDeferredTool{name: "mcp__srv__beta", desc: "beta"})
@@ -368,9 +368,8 @@ func TestPartitionToolsActiveNothingHiddenEmptyReminder(t *testing.T) {
 	if !exposedNames["tool_search"] {
 		t.Fatalf("expected tool_search exposed on active path, got %#v", exposed)
 	}
-	// No hidden tools means no dynamic discovery text is needed.
-	if reminder != "" {
-		t.Fatalf("expected empty discovery text when nothing is hidden, got %q", reminder)
+	if !strings.Contains(reminder, "mcp__srv__alpha") || !strings.Contains(reminder, "mcp__srv__beta") {
+		t.Fatalf("stable catalog must retain all deferred names after loading, got %q", reminder)
 	}
 }
 

internal/agent/denial_test.go

@@ -11,7 +11,7 @@ import (
 func TestExecuteToolCallCategorizesFilteredDenial(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	result, _ := executeToolCall(context.Background(), registry, ToolCall{ID: "c1", Name: "read_file", Arguments: `{"path":"x"}`}, PermissionModeAuto, Options{
 		DisabledTools: []string{"read_file"},

internal/agent/export_test.go

@@ -0,0 +1,41 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package agent
+
+import (
+	"github.com/Gitlawb/zero/internal/tools"
+	"github.com/Gitlawb/zero/internal/zeroruntime"
+)
+
+func equalStringSlices(left []string, right []string) bool {
+	if len(left) != len(right) {
+		return false
+	}
+	for index := range left {
+		if left[index] != right[index] {
+			return false
+		}
+	}
+	return true
+}
+
+// parsePreservedState recovers the plan + skills from a prior summary's preserved
+// block. JSON escaping makes this lossless even when a skill body contains
+// markdown headings, code fences, or quotes. Returns ("", nil) when absent or
+// malformed.
+func parsePreservedState(summaryContent string) (string, []skillEntry) {
+	state := parsePreservedStateBlock(summaryContent)
+	return state.Plan, preservedSkillsToEntries(state.Skills)
+}
+
+// partitionTools builds the per-turn advertised tool list and optional
+// tool_search discovery text. INACTIVE (DeferThreshold <= 0 or the eligible count is
+// below it): every visible tool is exposed with its full schema EXCEPT tool_search
+// (dropped so it is never advertised when it cannot help), and the discovery text is
+// empty — byte-identical to the pre-deferral output. ACTIVE: a deferred-eligible
+// tool is exposed only when loaded[name]; otherwise it is hidden and searchable
+// through tool_search. Non-deferred tools (including tool_search) are always
+// exposed. The exposed slice is alpha-sorted by name, matching the legacy order
+// so the inactive path is stable.
+func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) {
+	return partitionToolsCached(registry, permissionMode, options, loaded, nil)
+}

internal/agent/guardrails_test.go

@@ -138,7 +138,7 @@ func TestRunResetsEmptyTurnCounterOnToolCall(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -260,7 +260,7 @@ func TestRunInjectsPlanNotCalledReminderForMultiStepTask(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -291,7 +291,7 @@ func TestRunDoesNotInjectPlanReminderForTrivialTask(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -319,7 +319,7 @@ func TestRunDoesNotInjectNotCalledReminderWhenPlanUsed(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	registry.Register(tools.NewUpdatePlanTool())
 
 	provider := &mockProvider{
@@ -349,7 +349,7 @@ func TestRunInjectsStalePlanReminderAfterManyToolCalls(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	registry.Register(tools.NewUpdatePlanTool())
 
 	// Turn 1 calls update_plan (so the not-called reminder never triggers), then
@@ -383,7 +383,7 @@ func TestRunStalePlanReminderIsOneShotPerInterval(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	registry.Register(tools.NewUpdatePlanTool())
 
 	turns := [][]zeroruntime.StreamEvent{
@@ -418,7 +418,7 @@ func TestRunInjectsToolOnlyProgressReminder(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, root+"/notes.txt", "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	turns := make([][]zeroruntime.StreamEvent, 0, toolOnlyProgressReminderAt+1)
 	for i := 0; i < toolOnlyProgressReminderAt; i++ {

internal/agent/loop.go

@@ -267,10 +267,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 			})
 		}
 
-		// Build the per-turn tool list first so proactive compaction can include
-		// the tool-definition tokens (they ride on every request) in its estimate.
-		// partitionTools depends only on registry/permissions/options/loaded, not on
-		// the messages, so computing it before compaction is safe.
 		// Build the per-turn tool list first so proactive compaction can include
 		// the tool-definition tokens (they ride on every request) in its estimate.
 		// partitionTools depends only on registry/permissions/options/loaded, not on
@@ -660,6 +656,13 @@ 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
@@ -713,6 +716,16 @@ 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
@@ -723,11 +736,13 @@ 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)
@@ -751,6 +766,7 @@ 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
@@ -769,6 +785,10 @@ 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
@@ -1080,6 +1100,9 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 			}, nil
 		}
 	}
+	if isShellCommandTool(call.Name) {
+		normalizeNoopAdditionalPermissions(args, options.Cwd, options.Sandbox)
+	}
 	// tool_search is the gateway to the allowlisted deferred tools, so a non-empty
 	// EnabledTools allowlist that omits it must NOT reject the call — otherwise the
 	// discovery tool exposed to the model rejects at dispatch time (an inescapable
@@ -1358,7 +1381,8 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 		Cwd:               options.Cwd,
 		// Per-session file version tracker so write_file/edit_file refuse to clobber
 		// a file that changed on disk outside Zero since it was last read.
-		FileTracker: options.FileTracker,
+		FileTracker:                options.FileTracker,
+		DeferFileObservationCommit: true,
 		// Post-edit diagnostics are NOT forwarded to the tools: they used to run
 		// synchronously inside edit_file/write_file and block every mutating tool
 		// call on the language server (≥300ms debounce floor, 10s cap). The loop
@@ -1410,6 +1434,7 @@ func executeToolCall(ctx context.Context, registry *tools.Registry, call ToolCal
 			result = registry.RebudgetAfterHook(call.Name, args, result)
 		}
 	}
+	result = registry.CommitFileObservation(result, options.FileTracker)
 	// Stamp the sandbox risk classification for this EXECUTED call so run-policy
 	// observers can see the risk of an allowed mutation. Prefer the preflight
 	// decision's classification when the sandbox evaluated the call; otherwise
@@ -1435,6 +1460,7 @@ 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,
@@ -1621,9 +1647,15 @@ func sandboxRestrictedShellRetryCandidate(call ToolCall, args map[string]any, re
 
 func sandboxDeniedShellResult(result tools.Result) bool {
 	if result.ExecutionOutcome != nil {
-		// Typed denials must be handled by their exact capability path. Never
-		// turn a structured narrow denial into the legacy unrestricted retry.
-		return false
+		// Only a typed platform denial that explicitly requests unrestricted
+		// recovery may enter the legacy retry path. Narrow capability denials
+		// remain on their exact capability path.
+		denial := result.ExecutionOutcome.Denial
+		return denial != nil &&
+			denial.Source == execution.DenialSourcePlatformSandbox &&
+			denial.Capability.Kind == execution.CapabilityUnrestricted &&
+			denial.Recoverable &&
+			denial.NextAction == execution.DenialNextActionRequestApproval
 	}
 	return result.Status == tools.StatusError && result.Meta[tools.SandboxLikelyDeniedMeta] == "true"
 }
@@ -1688,20 +1720,21 @@ func sandboxRestrictionRetryReason(result tools.Result) string {
 
 func runToolForNetworkRetry(ctx context.Context, registry *tools.Registry, name string, toolCallID string, args map[string]any, permissionMode PermissionMode, options Options, progressCallback func(streamjson.Event)) tools.Result {
 	return registry.RunWithOptions(ctx, name, cloneArgs(args), tools.RunOptions{
-		PermissionGranted: true,
-		PermissionMode:    string(permissionMode),
-		Autonomy:          options.Autonomy,
-		Sandbox:           options.Sandbox,
-		ToolCallID:        toolCallID,
-		SessionID:         options.SessionID,
-		Model:             options.Model,
-		ReasoningEffort:   options.ReasoningEffort,
-		Depth:             options.Depth,
-		Cwd:               options.Cwd,
-		FileTracker:       options.FileTracker,
-		EnabledTools:      options.EnabledTools,
-		DisabledTools:     options.DisabledTools,
-		Progress:          progressCallback,
+		PermissionGranted:          true,
+		PermissionMode:             string(permissionMode),
+		Autonomy:                   options.Autonomy,
+		Sandbox:                    options.Sandbox,
+		ToolCallID:                 toolCallID,
+		SessionID:                  options.SessionID,
+		Model:                      options.Model,
+		ReasoningEffort:            options.ReasoningEffort,
+		Depth:                      options.Depth,
+		Cwd:                        options.Cwd,
+		FileTracker:                options.FileTracker,
+		DeferFileObservationCommit: true,
+		EnabledTools:               options.EnabledTools,
+		DisabledTools:              options.DisabledTools,
+		Progress:                   progressCallback,
 	})
 }
 
@@ -1716,20 +1749,21 @@ func unsandboxedRetryArgs(args map[string]any) map[string]any {
 
 func runToolForUnsandboxedRetry(ctx context.Context, registry *tools.Registry, name string, toolCallID string, args map[string]any, permissionMode PermissionMode, options Options, progressCallback func(streamjson.Event)) tools.Result {
 	return registry.RunWithOptions(ctx, name, args, tools.RunOptions{
-		PermissionGranted: true,
-		PermissionMode:    string(permissionMode),
-		Autonomy:          options.Autonomy,
-		Sandbox:           options.Sandbox,
-		ToolCallID:        toolCallID,
-		SessionID:         options.SessionID,
-		Model:             options.Model,
-		ReasoningEffort:   options.ReasoningEffort,
-		Depth:             options.Depth,
-		Cwd:               options.Cwd,
-		FileTracker:       options.FileTracker,
-		EnabledTools:      options.EnabledTools,
-		DisabledTools:     options.DisabledTools,
-		Progress:          progressCallback,
+		PermissionGranted:          true,
+		PermissionMode:             string(permissionMode),
+		Autonomy:                   options.Autonomy,
+		Sandbox:                    options.Sandbox,
+		ToolCallID:                 toolCallID,
+		SessionID:                  options.SessionID,
+		Model:                      options.Model,
+		ReasoningEffort:            options.ReasoningEffort,
+		Depth:                      options.Depth,
+		Cwd:                        options.Cwd,
+		FileTracker:                options.FileTracker,
+		DeferFileObservationCommit: true,
+		EnabledTools:               options.EnabledTools,
+		DisabledTools:              options.DisabledTools,
+		Progress:                   progressCallback,
 	})
 }
 
@@ -2746,21 +2780,89 @@ func shellCommandAdditionalPermissionsRequested(args map[string]any) bool {
 	return strings.TrimSpace(value) == string(tools.SandboxPermissionsWithAdditionalPermissions)
 }
 
+// normalizeNoopAdditionalPermissions treats a provider-expanded empty or
+// already-covered additional_permissions object as if it had been omitted.
+// Some strict-schema providers materialize every optional property and may add
+// a workspace read that the default sandbox already grants; rejecting either
+// no-op makes the model retry an otherwise valid command. Requests that broaden
+// 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")
+		return
+	}
+	data, err := json.Marshal(raw)
+	if err != nil {
+		return
+	}
+	var profile sandbox.RequestPermissionProfile
+	if err := json.Unmarshal(data, &profile); err != nil {
+		return
+	}
+	normalized, err := sandbox.NormalizeRequestPermissionProfile(profile, basePath)
+	if err != nil {
+		return
+	}
+	noop := normalized.Empty()
+	if !noop && engine != nil {
+		if grantProfile, grantErr := sandbox.RequestPermissionGrantProfile(normalized); grantErr == nil {
+			noop = engine.CoversRequestPermissions(grantProfile)
+		}
+	}
+	if !noop {
+		return
+	}
+	delete(args, "additional_permissions")
+	if rawMode, ok := args["sandbox_permissions"].(string); ok &&
+		strings.TrimSpace(rawMode) == string(tools.SandboxPermissionsWithAdditionalPermissions) {
+		args["sandbox_permissions"] = string(tools.SandboxPermissionsUseDefault)
+	}
+}
+
+// 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) {
-		if _, exists := args["additional_permissions"]; exists {
-			return sandbox.RequestPermissionProfile{}, false, fmt.Errorf("additional_permissions requires sandbox_permissions set to %q", tools.SandboxPermissionsWithAdditionalPermissions)
+		// 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)
 		}
 		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. "+
-				`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)
+			"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)
 	}
 	data, err := json.Marshal(raw)
 	if err != nil {
@@ -2776,8 +2878,10 @@ func inlineAdditionalPermissionsProfile(args map[string]any, basePath string) (s
 	}
 	if normalized.Empty() {
 		return sandbox.RequestPermissionProfile{}, true, fmt.Errorf(
-			"additional_permissions must include at least one of network or file_system, for example " +
-				`{"network": {"enabled": true}} or {"file_system": {"write": ["/path"]}}`)
+			"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)
 	}
 	grantProfile, err := sandbox.RequestPermissionGrantProfile(normalized)
 	if err != nil {
@@ -2866,19 +2970,6 @@ func permissionActionFromSandbox(action sandbox.Action) PermissionAction {
 	}
 }
 
-// partitionTools builds the per-turn advertised tool list and optional
-// tool_search discovery text. INACTIVE (DeferThreshold <= 0 or the eligible count is
-// below it): every visible tool is exposed with its full schema EXCEPT tool_search
-// (dropped so it is never advertised when it cannot help), and the discovery text is
-// empty — byte-identical to the pre-deferral output. ACTIVE: a deferred-eligible
-// tool is exposed only when loaded[name]; otherwise it is hidden and searchable
-// through tool_search. Non-deferred tools (including tool_search) are always
-// exposed. The exposed slice is alpha-sorted by name, matching the legacy order
-// so the inactive path is stable.
-func partitionTools(registry *tools.Registry, permissionMode PermissionMode, options Options, loaded map[string]bool) ([]zeroruntime.ToolDefinition, string) {
-	return partitionToolsCached(registry, permissionMode, options, loaded, nil)
-}
-
 // partitionToolsCached is partitionTools with an optional per-tool definition
 // cache. The partitioning itself (visibility, deferral, ordering) is recomputed
 // every call — it must be, because a tool's deferred state can flip mid-run (e.g.
@@ -2949,22 +3040,20 @@ func partitionToolsCached(registry *tools.Registry, permissionMode PermissionMod
 	//   2. tool_search — always present, right after the stable block;
 	//   3. loaded deferred tools, alpha-sorted — APPENDED, so an unloaded->loaded
 	//      transition grows the tail instead of inserting into the eager block.
-	// (tool_search's discovery text still shrinks as tools load, so keeping it and
-	// the loaded tail AFTER the eager block preserves the eager tools' cache across a
-	// load; fully stabilizing the loader's own description is a scoped follow-up.)
+	// tool_search's compact catalog also stays byte-stable as tools load, so only
+	// the appended full-schema tail changes.
 	eager := make([]zeroruntime.ToolDefinition, 0, len(visible))
 	loadedTail := make([]zeroruntime.ToolDefinition, 0)
-	var hiddenTools []tools.Tool
+	var deferredCatalog []tools.Tool
 	for _, tool := range visible {
 		name := tool.Name()
 		if name == tools.ToolSearchToolName {
 			continue // added explicitly between the eager block and the loaded tail
 		}
 		if tools.IsDeferred(tool) {
+			deferredCatalog = append(deferredCatalog, tool)
 			if loaded[name] {
 				loadedTail = append(loadedTail, cachedRuntimeToolDefinition(defCache, tool))
-			} else {
-				hiddenTools = append(hiddenTools, tool)
 			}
 			continue
 		}
@@ -2977,15 +3066,16 @@ func partitionToolsCached(registry *tools.Registry, permissionMode PermissionMod
 		return loadedTail[left].Name < loadedTail[right].Name
 	})
 
-	discovery := ""
-	if len(hiddenTools) > 0 {
-		discovery = tools.BuildToolSearchDescription(hiddenTools)
-	}
+	// Keep the catalog byte-stable as tools load. tool_search already redirects
+	// requests for an exposed tool back to the current tool list, so retaining
+	// loaded names costs only compact catalog lines and avoids invalidating the
+	// provider cache at the loader definition on every load.
+	discovery := tools.BuildToolSearchDescription(deferredCatalog)
 
 	// tool_search is guaranteed runnable on the active path, so it is ALWAYS exposed
 	// — even when a non-empty EnabledTools allowlist omits it (the operator
 	// allowlisted the deferred tools, not the loader). It sits right after the stable
-	// eager block and carries the discovery list for still-hidden tools.
+	// eager block and carries the stable deferred-tool catalog.
 	description := loader.Description()
 	if discovery != "" {
 		description = discovery
@@ -3224,3 +3314,37 @@ 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/loop_test.go

@@ -64,6 +64,22 @@ func TestTypedExecutionOutcomeOverridesLegacySandboxHeuristics(t *testing.T) {
 		t.Fatal("narrow protected-metadata denial must not become an unrestricted retry")
 	}
 
+	nonRecoverableDenial := protectedDenial
+	nonRecoverableDenial.ExecutionOutcome = &execution.Outcome{
+		State: execution.StateDenied,
+		Kind:  execution.OutcomeEnforcementDenied,
+		Denial: &execution.Denial{
+			Capability:  execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
+			Source:      execution.DenialSourcePlatformSandbox,
+			Reason:      "sandbox enforcement failed closed",
+			Recoverable: false,
+			NextAction:  execution.DenialNextActionRequestApproval,
+		},
+	}
+	if sandboxRestrictedShellRetryCandidate(call, nil, nonRecoverableDenial, options) {
+		t.Fatal("non-recoverable sandbox denial must not become an unrestricted retry")
+	}
+
 	networkDenial := protectedDenial
 	networkDenial.ExecutionOutcome = &execution.Outcome{
 		State: execution.StateDenied,
@@ -288,6 +304,18 @@ func (tool *sandboxDeniedRetryTool) Run(_ context.Context, args map[string]any)
 	return tools.Result{
 		Status: tools.StatusError,
 		Output: "touch: cannot touch '/home/user/.npm/cache': Read-only file system",
+		ExecutionOutcome: &execution.Outcome{
+			State: execution.StateDenied,
+			Kind:  execution.OutcomeEnforcementDenied,
+			Exit:  &execution.Exit{Code: 1},
+			Denial: &execution.Denial{
+				Capability:  execution.Capability{Kind: execution.CapabilityUnrestricted, Scope: "host"},
+				Source:      execution.DenialSourcePlatformSandbox,
+				Reason:      "sandbox blocked command execution",
+				Recoverable: true,
+				NextAction:  execution.DenialNextActionRequestApproval,
+			},
+		},
 		Meta: map[string]string{
 			"exit_code":                    "1",
 			tools.SandboxLikelyDeniedMeta:  "true",
@@ -304,7 +332,8 @@ type sandboxDeniedExecCommandRetryTool struct {
 func (tool *sandboxDeniedExecCommandRetryTool) Name() string { return "exec_command" }
 
 type sandboxNetworkDeniedRetryTool struct {
-	calls []map[string]any
+	calls                      []map[string]any
+	deferFileObservationCommit []bool
 }
 
 func (tool *sandboxNetworkDeniedRetryTool) Name() string        { return "bash" }
@@ -327,6 +356,7 @@ func (tool *sandboxNetworkDeniedRetryTool) Run(ctx context.Context, args map[str
 }
 func (tool *sandboxNetworkDeniedRetryTool) RunWithOptions(ctx context.Context, args map[string]any, options tools.RunOptions) tools.Result {
 	tool.calls = append(tool.calls, cloneArgs(args))
+	tool.deferFileObservationCommit = append(tool.deferFileObservationCommit, options.DeferFileObservationCommit)
 	if shellNetworkAllowed(ctx, options.Sandbox, args) {
 		return tools.Result{Status: tools.StatusOK, Output: "server started"}
 	}
@@ -369,7 +399,8 @@ func agentNativeBackendStub() sandbox.Backend {
 }
 
 type sandboxNamespaceLimitedRetryTool struct {
-	calls []map[string]any
+	calls                      []map[string]any
+	deferFileObservationCommit []bool
 }
 
 func (tool *sandboxNamespaceLimitedRetryTool) Name() string        { return "bash" }
@@ -388,8 +419,12 @@ func (tool *sandboxNamespaceLimitedRetryTool) Parameters() tools.Schema {
 func (tool *sandboxNamespaceLimitedRetryTool) Safety() tools.Safety {
 	return tools.Safety{SideEffect: tools.SideEffectShell, Permission: tools.PermissionPrompt, Reason: "runs shell commands"}
 }
-func (tool *sandboxNamespaceLimitedRetryTool) Run(_ context.Context, args map[string]any) tools.Result {
+func (tool *sandboxNamespaceLimitedRetryTool) Run(ctx context.Context, args map[string]any) tools.Result {
+	return tool.RunWithOptions(ctx, args, tools.RunOptions{})
+}
+func (tool *sandboxNamespaceLimitedRetryTool) RunWithOptions(_ context.Context, args map[string]any, options tools.RunOptions) tools.Result {
 	tool.calls = append(tool.calls, cloneArgs(args))
+	tool.deferFileObservationCommit = append(tool.deferFileObservationCommit, options.DeferFileObservationCommit)
 	if args["sandbox_permissions"] == string(tools.SandboxPermissionsRequireEscalated) {
 		return tools.Result{Status: tools.StatusOK, Output: "stdout:\nUSER PID COMMAND\nanaxy 42 firefox\nanaxy 43 Discord"}
 	}
@@ -455,6 +490,9 @@ func TestRunRetriesShellUnsandboxedAfterSandboxNamespaceLimitedOutput(t *testing
 	if retryTool.calls[1]["sandbox_permissions"] != string(tools.SandboxPermissionsRequireEscalated) {
 		t.Fatalf("retry args = %#v, want require_escalated", retryTool.calls[1])
 	}
+	if len(retryTool.deferFileObservationCommit) != 2 || !retryTool.deferFileObservationCommit[1] {
+		t.Fatalf("retry defer flags = %#v, want retry observation deferred", retryTool.deferFileObservationCommit)
+	}
 	if len(requests) != 1 {
 		t.Fatalf("permission requests = %#v, want one unsandboxed retry approval", requests)
 	}
@@ -606,6 +644,9 @@ func TestRunRetriesNetworkDeniedShellWithNetworkGrant(t *testing.T) {
 	if _, escalated := retryTool.calls[1]["sandbox_permissions"]; escalated {
 		t.Fatalf("network retry must stay sandboxed, got retry args %#v", retryTool.calls[1])
 	}
+	if len(retryTool.deferFileObservationCommit) != 2 || !retryTool.deferFileObservationCommit[1] {
+		t.Fatalf("retry defer flags = %#v, want retry observation deferred", retryTool.deferFileObservationCommit)
+	}
 	if len(requests) != 1 || requests[0].Reason != sandbox.ReasonNetworkBlocked {
 		t.Fatalf("permission requests = %#v, want one network approval", requests)
 	}
@@ -863,9 +904,6 @@ func TestRunReportsTruncationFinishReason(t *testing.T) {
 	if result.FinishReason != zeroruntime.FinishReasonLength {
 		t.Fatalf("FinishReason = %q, want %q", result.FinishReason, zeroruntime.FinishReasonLength)
 	}
-	if !result.Truncated() {
-		t.Fatal("Truncated() = false, want true for a length-capped response")
-	}
 	if result.TruncationNotice() == "" {
 		t.Fatal("TruncationNotice() empty for a truncated response")
 	}
@@ -883,7 +921,7 @@ func TestRunNormalCompletionIsNotTruncated(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	if result.Truncated() || result.TruncationNotice() != "" {
+	if result.TruncationNotice() != "" {
 		t.Fatalf("normal completion reported as truncated: reason=%q", result.FinishReason)
 	}
 }
@@ -960,7 +998,7 @@ func TestRunEmitsUsageEvents(t *testing.T) {
 func TestRunAdvertisesRuntimeToolDefinitions(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{{
 			{Type: zeroruntime.StreamEventText, Content: "done"},
@@ -1167,9 +1205,9 @@ func TestRunRequestsPermissionBeforeWebSearchExecution(t *testing.T) {
 func TestRunFiltersAdvertisedTools(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
-	registry.Register(tools.NewGrepTool(root))
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
+	registry.Register(tools.NewScopedGrepTool(root, nil))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{{
 			{Type: zeroruntime.StreamEventText, Content: "done"},
@@ -1197,7 +1235,7 @@ func TestRunFiltersAdvertisedTools(t *testing.T) {
 func TestRunRejectsFilteredToolCalls(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -1234,7 +1272,7 @@ func TestRunRejectsFilteredToolCalls(t *testing.T) {
 func TestRunRejectsToolCallsOutsideEnabledList(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -1272,7 +1310,7 @@ func TestRunExecutesToolCallThroughRegistry(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha\nbeta\n")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -1356,7 +1394,7 @@ func TestRunPreservesRequestPrefixAcrossTurns(t *testing.T) {
 func TestRunSanitizesMalformedToolCallArgumentsBeforeRetry(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -1424,7 +1462,7 @@ func TestRunRecoversFirstObjectFromConcatenatedToolArgs(t *testing.T) {
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -1471,8 +1509,8 @@ func TestRunDefersSelfCorrectFeedbackUntilAfterToolBatch(t *testing.T) {
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -1547,7 +1585,7 @@ func TestRunBatchesSelfCorrectOncePerTurn(t *testing.T) {
 	// after a later call in the same turn supersedes it.
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -1604,7 +1642,7 @@ func TestRunBatchesSelfCorrectOncePerTurn(t *testing.T) {
 func TestRunDeniesPromptToolWithoutUnsafePermission(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write denied")
 	var permissionEvents []PermissionEvent
 
@@ -1647,7 +1685,7 @@ func TestRunDeniesPromptToolWithoutUnsafePermission(t *testing.T) {
 func TestRunRequestsPromptToolPermissionBeforeExecution(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write approved")
 	var requests []PermissionRequest
 	var permissionEvents []PermissionEvent
@@ -1703,7 +1741,7 @@ func TestRunRequestsPromptToolPermissionBeforeExecution(t *testing.T) {
 func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write done")
 	var permissionEvents []PermissionEvent
 
@@ -1752,7 +1790,7 @@ func TestRunAllowsWorkspaceWriteWithoutPromptWhenSandboxPolicyPermits(t *testing
 func TestRunDeniesPromptToolWhenPermissionRequestDenied(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write denied")
 	var requests []PermissionRequest
 	var permissionEvents []PermissionEvent
@@ -1800,7 +1838,7 @@ func TestRunDeniesPromptToolWhenPermissionRequestDenied(t *testing.T) {
 func TestRunAbortsWhenPermissionRequestCanceled(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write should not continue")
 	var permissionEvents []PermissionEvent
 
@@ -1896,7 +1934,7 @@ func TestRunPersistsAlwaysAllowPermissionDecision(t *testing.T) {
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write approved")
 	var permissionEvents []PermissionEvent
 	policy := sandbox.DefaultPolicy()
@@ -1968,7 +2006,7 @@ func TestRunSessionAllowSkipsMatchingPromptWithoutPersistentGrant(t *testing.T)
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -2049,7 +2087,7 @@ func TestRunSessionAllowSkipsMatchingPromptWithoutPersistentGrant(t *testing.T)
 func TestRunCommandPrefixApprovalSkipsLaterMatchingBashPrompt(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -2251,7 +2289,7 @@ func TestRunPersistentCommandPrefixApprovalSkipsFutureSessionPrompt(t *testing.T
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	policy := sandbox.DefaultPolicy()
 	policy.Network = sandbox.NetworkAllow
 
@@ -2358,7 +2396,7 @@ func TestRunPersistentCommandPrefixStillPromptsForNetwork(t *testing.T) {
 		t.Fatalf("seed command prefix: %v", err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -2407,7 +2445,11 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) {
 	root := t.TempDir()
 	command := "PATH=.:$PATH curl https://example.com"
 	if runtime.GOOS == "windows" {
-		command = "set PATH=.;%PATH% && curl https://example.com"
+		if windowsTestUsesPowerShell() {
+			command = `$env:PATH = '.;' + $env:PATH; curl.cmd https://example.com`
+		} else {
+			command = "set PATH=.;%PATH% && curl https://example.com"
+		}
 		fakeCurl := filepath.Join(root, "curl.cmd")
 		if err := os.WriteFile(fakeCurl, []byte("@echo fake curl %*\r\n"), 0o755); err != nil {
 			t.Fatal(err)
@@ -2419,7 +2461,7 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) {
 		}
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -2486,7 +2528,7 @@ func TestRunApprovedNetworkBashPromptAppliesTurnNetworkGrant(t *testing.T) {
 func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -2528,13 +2570,17 @@ func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) {
 
 func TestRunPromptsForDestructiveShellInsteadOfSandboxDeny(t *testing.T) {
 	root := t.TempDir()
+	command := "echo rm -rf /"
+	if runtime.GOOS == "windows" && windowsTestUsesPowerShell() {
+		command = `Write-Output 'rm -rf /'`
+	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
 				{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "bash"},
-				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":"echo rm -rf /"}`},
+				{Type: zeroruntime.StreamEventToolCallDelta, ToolCallID: "call-1", ArgumentsFragment: `{"command":` + quoteJSONString(command) + `}`},
 				{Type: zeroruntime.StreamEventToolCallEnd, ToolCallID: "call-1"},
 				{Type: zeroruntime.StreamEventDone},
 			},
@@ -2598,7 +2644,7 @@ func TestRunAlwaysAllowWithoutSandboxStillAllowsCall(t *testing.T) {
 	// prior code denied it because persistPermissionGrant errors when Sandbox==nil.
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write approved")
 	var permissionEvents []PermissionEvent
 
@@ -2630,6 +2676,12 @@ func TestRunAlwaysAllowWithoutSandboxStillAllowsCall(t *testing.T) {
 	}
 }
 
+func windowsTestUsesPowerShell() bool {
+	executable, _ := tools.HostShellCommand("")
+	name := strings.ToLower(filepath.Base(executable))
+	return strings.Contains(name, "powershell") || strings.HasPrefix(name, "pwsh")
+}
+
 func containsPermissionDecision(decisions []PermissionDecisionAction, want PermissionDecisionAction) bool {
 	for _, decision := range decisions {
 		if decision == want {
@@ -2662,7 +2714,7 @@ func TestRunCancellationPreservesContextCanceledIdentity(t *testing.T) {
 func TestRunGrantsPromptToolInUnsafeMode(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write done")
 	var permissionEvents []PermissionEvent
 
@@ -2714,7 +2766,7 @@ func TestRunEmitsPermissionEventForPersistentSandboxGrant(t *testing.T) {
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("write done")
 	var permissionEvents []PermissionEvent
 
@@ -2893,7 +2945,7 @@ func TestRunAppliesSandboxEvenInUnsafeMode(t *testing.T) {
 	root := t.TempDir()
 	outside := filepath.Join(tempDirOutsideDefaultTemp(t), "escape.txt")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWritePathThenAnswer(outside, "sandbox handled")
 	var permissionEvents []PermissionEvent
 
@@ -2942,7 +2994,7 @@ func TestRunStopsAfterMaxTurns(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{{
 			{Type: zeroruntime.StreamEventToolCallStart, ToolCallID: "call-1", ToolName: "read_file"},
@@ -2972,7 +3024,7 @@ func TestRunRequestsFinalAnswerAfterMaxTurns(t *testing.T) {
 	root := t.TempDir()
 	writeAgentTestFile(t, filepath.Join(root, "notes.txt"), "alpha")
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -3163,22 +3215,17 @@ func TestBuildSystemPromptInjectsHostShellContext(t *testing.T) {
 		t.Fatalf("expected operating system in environment block, got %q", prompt)
 	}
 	if runtime.GOOS == "windows" {
-		for _, want := range []string{"Windows cmd.exe syntax", "cwd argument", "MSYS binaries", "grep", "require_escalated", "use double quotes around the value"} {
-			if !strings.Contains(prompt, want) {
-				t.Fatalf("expected Windows shell guidance to mention %q, got %q", want, prompt)
-			}
+		wants := []string{"workdir/cwd", "MSYS binaries"}
+		if windowsTestUsesPowerShell() {
+			wants = append(wants, "PowerShell", "Get-ChildItem", "Select-String", "$env:NAME", "require_escalated")
+		} else {
+			wants = append(wants, "cmd.exe", "double quotes")
 		}
-		// The examples must themselves use the safe (double-quoted) form; a
-		// single-quoted example here would teach the model the exact syntax
-		// that fails under cmd.exe.
-		for _, want := range []string{`--jq ".a | b"`, `-run "A|B"`} {
+		for _, want := range wants {
 			if !strings.Contains(prompt, want) {
-				t.Fatalf("expected double-quoted example %q in Windows shell guidance, got %q", want, prompt)
+				t.Fatalf("expected selected Windows shell guidance to mention %q, got %q", want, prompt)
 			}
 		}
-		if strings.Contains(prompt, `'.a | b'`) || strings.Contains(prompt, `'A|B'`) {
-			t.Fatalf("Windows shell guidance must not show the unsafe single-quoted form, got %q", prompt)
-		}
 	} else if !strings.Contains(prompt, "/bin/sh syntax") {
 		t.Fatalf("expected POSIX shell guidance in prompt, got %q", prompt)
 	}
@@ -3231,7 +3278,7 @@ func TestBuildSystemPromptAllowsSpecModeOverride(t *testing.T) {
 func TestSpecDraftAdvertisesOnlySafeDraftTools(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	for _, tool := range tools.CoreTools(root) {
+	for _, tool := range tools.CoreToolsScoped(root, nil) {
 		registry.Register(tool)
 	}
 	specmode.RegisterDraftTools(registry, root, nil)
@@ -3268,7 +3315,7 @@ func TestSpecDraftAdvertisesOnlySafeDraftTools(t *testing.T) {
 func TestSpecDraftDeniesHiddenToolCalls(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewWriteFileTool(root))
+	registry.Register(tools.NewScopedWriteFileTool(root, nil))
 	provider := providerCallingWriteFileThenAnswer("done")
 
 	result, err := Run(context.Background(), "draft", provider, Options{
@@ -3301,7 +3348,7 @@ func TestSpecDraftDeniesHiddenToolCalls(t *testing.T) {
 func TestSpecDraftDeniesBashToolCalls(t *testing.T) {
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewBashTool(root))
+	registry.Register(tools.NewScopedBashTool(root, nil))
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
 			{
@@ -3442,7 +3489,7 @@ func TestRunSurfacesDroppedToolCallAlongsideValidCall(t *testing.T) {
 		t.Fatal(err)
 	}
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewScopedReadFileTool(root, nil))
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{

internal/agent/output_budget_propagation_test.go

@@ -3,6 +3,7 @@ package agent
 import (
 	"context"
 	"os"
+	"path/filepath"
 	"strconv"
 	"strings"
 	"testing"
@@ -19,6 +20,16 @@ type propagationOutputTool struct {
 
 func TestOutputBudgetHookHelperProcess(t *testing.T) {
 	for index, arg := range os.Args {
+		if arg == "--zero-output-budget-hook-repeat" && index+1 < len(os.Args) {
+			repeats, err := strconv.Atoi(os.Args[index+1])
+			if err != nil {
+				os.Exit(2)
+			}
+			if _, err := os.Stdout.WriteString(strings.Repeat("hook feedback ", repeats)); err != nil {
+				os.Exit(2)
+			}
+			os.Exit(0)
+		}
 		if arg != "--zero-output-budget-hook" || index+1 >= len(os.Args) {
 			continue
 		}
@@ -30,25 +41,102 @@ func TestOutputBudgetHookHelperProcess(t *testing.T) {
 }
 
 func largeOutputBudgetHookDispatcher() *hooks.Dispatcher {
-	feedback := strings.Repeat("hook feedback ", 200)
+	return outputBudgetHookDispatcherFor("propagation_output", 200)
+}
+
+func outputBudgetHookDispatcherFor(toolName string, repeats int) *hooks.Dispatcher {
+	feedback := strings.Repeat("hook feedback ", repeats)
+	hookArgs := []string{
+		"-test.run=TestOutputBudgetHookHelperProcess",
+		"--",
+		"--zero-output-budget-hook",
+		feedback,
+	}
+	if repeats > 1_000 {
+		hookArgs = []string{
+			"-test.run=TestOutputBudgetHookHelperProcess",
+			"--",
+			"--zero-output-budget-hook-repeat",
+			strconv.Itoa(repeats),
+		}
+	}
 	return hooks.NewDispatcher(hooks.DispatcherOptions{Config: hooks.Config{
 		Enabled: true,
 		Hooks: []hooks.Definition{{
 			ID:      "large-feedback",
 			Event:   hooks.EventAfterTool,
-			Matcher: "propagation_output",
+			Matcher: toolName,
 			Command: os.Args[0],
-			Args: []string{
-				"-test.run=TestOutputBudgetHookHelperProcess",
-				"--",
-				"--zero-output-budget-hook",
-				feedback,
-			},
+			Args:    hookArgs,
 			Enabled: true,
 		}},
 	}})
 }
 
+func TestExecuteToolCallCommitsReadObservationAfterFinalBoundary(t *testing.T) {
+	root := t.TempDir()
+	targetPath := filepath.Join(root, "target.txt")
+	if err := os.WriteFile(targetPath, []byte("known content\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	linkPath := filepath.Join(root, "small.txt")
+	if err := os.Symlink(targetPath, linkPath); err != nil {
+		t.Skipf("symlink unavailable: %v", err)
+	}
+	resolvedPath, err := filepath.EvalSymlinks(linkPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	tracker := tools.NewFileTracker()
+
+	result, abortErr := executeToolCall(context.Background(), registry, ToolCall{
+		ID: "read-small", Name: "read_file", Arguments: `{"path":"small.txt"}`,
+	}, PermissionModeAuto, Options{Cwd: root, FileTracker: tracker})
+	if abortErr != nil || result.Status != tools.StatusOK {
+		t.Fatalf("executeToolCall: result=%#v err=%v", result, abortErr)
+	}
+	if !tracker.SeenWhole(resolvedPath) {
+		t.Fatal("unmodified final read output was not committed as observed")
+	}
+}
+
+func TestExecuteToolCallDoesNotCommitReadObservationBeforeHookBudget(t *testing.T) {
+	t.Setenv("ZERO_TOOL_OUTPUT_CEILING_TOKENS", "80")
+	root := t.TempDir()
+	targetPath := filepath.Join(root, "target.txt")
+	if err := os.WriteFile(targetPath, []byte("known content\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	linkPath := filepath.Join(root, "small.txt")
+	if err := os.Symlink(targetPath, linkPath); err != nil {
+		t.Skipf("symlink unavailable: %v", err)
+	}
+	resolvedPath, err := filepath.EvalSymlinks(linkPath)
+	if err != nil {
+		t.Fatal(err)
+	}
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	tracker := tools.NewFileTracker()
+
+	result, abortErr := executeToolCall(context.Background(), registry, ToolCall{
+		ID: "read-hook", Name: "read_file", Arguments: `{"path":"small.txt"}`,
+	}, PermissionModeAuto, Options{
+		Cwd: root, FileTracker: tracker, Hooks: outputBudgetHookDispatcherFor("read_file", 20_000),
+	})
+	if abortErr != nil {
+		t.Fatalf("executeToolCall: %v", abortErr)
+	}
+	if !result.Truncated {
+		t.Fatalf("precondition: hook feedback was not truncated: %#v", result)
+	}
+	if tracker.SeenWhole(resolvedPath) {
+		t.Fatal("read observation committed before hook output was budgeted")
+	}
+}
+
 func (tool propagationOutputTool) Name() string        { return "propagation_output" }
 func (tool propagationOutputTool) Description() string { return "returns output for propagation tests" }
 func (tool propagationOutputTool) Parameters() tools.Schema {

internal/agent/partition_cache_stable_test.go

@@ -14,7 +14,7 @@ func TestPartitionToolsActiveAppendsLoadedToolAfterEagerBlock(t *testing.T) {
 	// "read_file".
 	root := t.TempDir()
 	registry := tools.NewRegistry()
-	registry.Register(tools.NewReadFileTool(root)) // non-deferred, "read_file"
+	registry.Register(tools.NewScopedReadFileTool(root, nil)) // non-deferred, "read_file"
 	registry.Register(fakeToolSearchTool{})
 	registry.Register(fakeDeferredTool{name: "mcp__srv__alpha", desc: "alpha"})
 	registry.Register(fakeDeferredTool{name: "mcp__srv__beta", desc: "beta"})

internal/agent/profile_controller.go

@@ -91,7 +91,7 @@ func (c *profileController) maybeEscalate() (PostureEscalation, bool) {
 	if c.policy == nil || c.escalated {
 		return PostureEscalation{}, false
 	}
-	if !(c.failureTripped || c.riskTripped || c.scTripped || c.uncertainTrip) {
+	if !c.failureTripped && !c.riskTripped && !c.scTripped && !c.uncertainTrip {
 		return PostureEscalation{}, false
 	}
 	c.escalated = true

internal/agent/prune.go

@@ -121,3 +121,67 @@ func toolNameForResult(messages []zeroruntime.Message, index int) string {
 func isPrunedPlaceholder(content string) bool {
 	return strings.HasPrefix(strings.TrimSpace(content), "[pruned ")
 }
+
+var supersedableReadTools = map[string]bool{
+	"read_file":          true,
+	"read_minified_file": true,
+	"grep":               true,
+	"glob":               true,
+	"list_directory":     true,
+}
+
+// pruneSupersededReadResults removes only older byte-identical results of the
+// same read call. It runs when context pressure already requires a rewrite, so
+// it does not trade a warm provider-cache prefix for a small apparent saving.
+func pruneSupersededReadResults(messages []zeroruntime.Message) ([]zeroruntime.Message, int) {
+	type seenResultKey struct {
+		call    string
+		content string
+	}
+	seen := map[seenResultKey]struct{}{}
+	out := messages
+	reclaimed := 0
+	copied := false
+
+	for index := len(messages) - 1; index >= 0; index-- {
+		message := messages[index]
+		if message.Role != zeroruntime.MessageRoleTool || isPrunedPlaceholder(message.Content) {
+			continue
+		}
+		call, ok := toolCallForResult(messages, index)
+		if !ok || !supersedableReadTools[call.Name] {
+			continue
+		}
+		key := seenResultKey{
+			call:    call.Name + "\x00" + strings.TrimSpace(call.Arguments),
+			content: message.Content,
+		}
+		if _, exists := seen[key]; exists {
+			placeholder := fmt.Sprintf("[superseded identical %s result — the newer result remains in context]", call.Name)
+			if !copied {
+				out = append([]zeroruntime.Message(nil), messages...)
+				copied = true
+			}
+			out[index].Content = placeholder
+			reclaimed += max(0, ApproxTextTokens(message.Content)-ApproxTextTokens(placeholder))
+			continue
+		}
+		seen[key] = struct{}{}
+	}
+	return out, reclaimed
+}
+
+func toolCallForResult(messages []zeroruntime.Message, resultIndex int) (zeroruntime.ToolCall, bool) {
+	id := messages[resultIndex].ToolCallID
+	if id == "" {
+		return zeroruntime.ToolCall{}, false
+	}
+	for index := resultIndex - 1; index >= 0; index-- {
+		for _, call := range messages[index].ToolCalls {
+			if call.ID == id {
+				return call, true
+			}
+		}
+	}
+	return zeroruntime.ToolCall{}, false
+}

internal/agent/prune_test.go

@@ -25,6 +25,13 @@ func toolCallMsg(id, name string) zeroruntime.Message {
 	}
 }
 
+func toolCallWithArgs(id, name, args string) zeroruntime.Message {
+	return zeroruntime.Message{
+		Role:      zeroruntime.MessageRoleAssistant,
+		ToolCalls: []zeroruntime.ToolCall{{ID: id, Name: name, Arguments: args}},
+	}
+}
+
 func TestPruneSkipsSmallSessions(t *testing.T) {
 	// Total reclaimable is under the gate → no pruning.
 	msgs := []zeroruntime.Message{
@@ -120,3 +127,69 @@ func TestPruneDoesNotMutateInput(t *testing.T) {
 		t.Fatal("pruneStaleToolOutput must not mutate the caller's slice")
 	}
 }
+
+func TestPruneSupersededReadResultsOnlyDropsIdenticalOlderReads(t *testing.T) {
+	body := strings.Repeat("same source line\n", 1000)
+	messages := []zeroruntime.Message{
+		toolCallWithArgs("old", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "old", Content: body},
+		toolCallWithArgs("different", "read_file", `{"path":"b.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "different", Content: body},
+		toolCallWithArgs("new", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "new", Content: body},
+	}
+	out, reclaimed := pruneSupersededReadResults(messages)
+	if reclaimed <= 0 || !strings.Contains(out[1].Content, "superseded identical") {
+		t.Fatalf("older duplicate was not pruned: reclaimed=%d output=%q", reclaimed, out[1].Content)
+	}
+	if out[3].Content != body || out[5].Content != body {
+		t.Fatal("different call and newest result must remain exact")
+	}
+	if messages[1].Content != body {
+		t.Fatal("input slice was mutated")
+	}
+}
+
+func TestPruneSupersededReadResultsKeepsChangedAndMutatingResults(t *testing.T) {
+	messages := []zeroruntime.Message{
+		toolCallWithArgs("r1", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "r1", Content: "version one"},
+		toolCallWithArgs("r2", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "r2", Content: "version two"},
+		toolCallWithArgs("e1", "edit_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "e1", Content: "edited"},
+		toolCallWithArgs("e2", "edit_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "e2", Content: "edited"},
+	}
+	out, reclaimed := pruneSupersededReadResults(messages)
+	if reclaimed != 0 {
+		t.Fatalf("unsafe results were pruned: %d", reclaimed)
+	}
+	for index := range messages {
+		if out[index].Content != messages[index].Content {
+			t.Fatalf("message %d changed unexpectedly", index)
+		}
+	}
+}
+
+func TestPruneSupersededReadResultsFindsMatchingBodyAcrossChangedRead(t *testing.T) {
+	bodyA := strings.Repeat("version a\n", 100)
+	bodyB := strings.Repeat("version b\n", 100)
+	messages := []zeroruntime.Message{
+		toolCallWithArgs("old-a", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "old-a", Content: bodyA},
+		toolCallWithArgs("middle-b", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "middle-b", Content: bodyB},
+		toolCallWithArgs("new-a", "read_file", `{"path":"a.go"}`),
+		{Role: zeroruntime.MessageRoleTool, ToolCallID: "new-a", Content: bodyA},
+	}
+
+	out, reclaimed := pruneSupersededReadResults(messages)
+
+	if reclaimed <= 0 || !strings.Contains(out[1].Content, "superseded identical") {
+		t.Fatalf("older matching body was not pruned across a changed result: reclaimed=%d output=%q", reclaimed, out[1].Content)
+	}
+	if out[3].Content != bodyB || out[5].Content != bodyA {
+		t.Fatal("changed middle result and newest matching result must remain exact")
+	}
+}

internal/agent/request_permissions_test.go

@@ -180,7 +180,7 @@ func TestInlineAdditionalPermissionsRequiresPromptUnderAutoAllow(t *testing.T) {
 		},
 	}
 	decision := &sandbox.Decision{Action: sandbox.ActionAllow, AutoAllowed: true}
-	if !shouldRequestPermission(tools.NewBashTool(t.TempDir()), args, false, decision) {
+	if !shouldRequestPermission(tools.NewScopedBashTool(t.TempDir(), nil), args, false, decision) {
 		t.Fatal("inline additional permissions must request approval even when a sandboxed shell would otherwise auto-allow")
 	}
 }

internal/agent/specialist_delegation_test.go

@@ -26,6 +26,16 @@ func TestSpecialistDelegationContext(t *testing.T) {
 	if strings.Contains(got, "nameless") {
 		t.Fatalf("nameless entry should be skipped: %q", got)
 	}
+	for _, want := range []string{"reduce total context", "Do not delegate small direct work", "bounded assignment"} {
+		if !strings.Contains(got, want) {
+			t.Fatalf("missing evidence-driven delegation guidance %q: %q", want, got)
+		}
+	}
+	for _, unwanted := range []string{"delegate to it proactively", "launch several specialists in parallel"} {
+		if strings.Contains(got, unwanted) {
+			t.Fatalf("retained broad delegation guidance %q: %q", unwanted, got)
+		}
+	}
 }
 
 func TestSystemPromptIncludesDelegationOnlyWithSpecialists(t *testing.T) {

internal/agent/system_prompt.go

@@ -11,6 +11,7 @@ import (
 
 	"github.com/Gitlawb/zero/internal/config"
 	"github.com/Gitlawb/zero/internal/repomap"
+	"github.com/Gitlawb/zero/internal/tools"
 	"github.com/Gitlawb/zero/internal/workspaceseed"
 )
 
@@ -178,21 +179,19 @@ func approvedCommandPrefixContext(options Options) string {
 	return "## Approved Command Prefixes\n\nThe following command prefixes have already been approved and do not need another permission prompt:\n" + strings.Join(lines, "\n")
 }
 
-// specialistDelegationContext nudges the orchestrator to offload read-heavy or
-// parallelizable work to a specialist sub-agent via the Task tool, keeping large
-// tool outputs out of the main context. It renders only when specialists are
-// known (which is only where the Task tool is actually registered), so a run with
-// no delegatable specialists produces the previous prompt unchanged.
+// specialistDelegationContext describes when a specialist earns its extra
+// context and coordination cost. It renders only when specialists are known
+// (which is only where the Task tool is actually registered), so a run with no
+// delegatable specialists produces the previous prompt unchanged.
 func specialistDelegationContext(options Options) string {
 	if len(options.Specialists) == 0 {
 		return ""
 	}
 	var b strings.Builder
 	b.WriteString("<specialists>\n")
-	b.WriteString("Delegate focused or read-heavy work to a specialist sub-agent with the Task tool instead of doing it inline. ")
-	b.WriteString("When a request matches a specialist's purpose, delegate to it proactively — you do not need the user to ask first. ")
-	b.WriteString("This keeps large tool outputs — searches, file dumps, multi-step exploration — out of your own context, so you stay fast and token-efficient. ")
-	b.WriteString("Prefer delegating codebase search and exploration; for independent subtasks, launch several specialists in parallel. Handle small, direct edits yourself.\n")
+	b.WriteString("Use the Task tool when a specialist's focused context, expertise, or an independent parallel subtask is likely to improve quality or reduce total context. ")
+	b.WriteString("Do not delegate small direct work, duplicate exploration, or split sequential steps merely because a specialist is available. ")
+	b.WriteString("When delegating, give a bounded assignment and consume its concise evidence instead of importing a long transcript.\n")
 	b.WriteString("Available specialists (call Task with the matching name when the task fits its purpose):\n")
 	for _, info := range options.Specialists {
 		name := strings.TrimSpace(info.Name)
@@ -310,11 +309,7 @@ func workspaceContext(cwd string) string {
 	b.WriteString("<environment>\n")
 	b.WriteString("Working directory: " + cwd + "\n")
 	b.WriteString("Operating system: " + runtime.GOOS + "\n")
-	if runtime.GOOS == "windows" {
-		b.WriteString("Shell syntax: Windows cmd.exe syntax for exec_command/bash tools. To put | & > < etc inside an arg value, use double quotes around the value, not single quotes (single quotes do not protect metachars in cmd.exe): gh --jq \".a | b\", go test -run \"A|B\". Do not pipe to or invoke POSIX coreutils from Git for Windows (usr\\bin head/grep/tail/cat/...): they are MSYS binaries and fail under the write-restricted sandbox; use native Zero tools (grep, read_file, list_directory, glob) or cmd.exe findstr/more instead, or sandbox_permissions require_escalated only when host-level execution is truly required. Prefer the workdir/cwd argument over cd when changing directories.\n")
-	} else {
-		b.WriteString("Shell syntax: /bin/sh syntax for exec_command/bash tools; prefer the workdir/cwd argument instead of cd when changing directories.\n")
-	}
+	b.WriteString(tools.HostShellEnvironmentGuidance() + "\n")
 	if branch := gitBranchForPrompt(cwd); branch != "" {
 		b.WriteString("Git branch: " + branch + "\n")
 	}

internal/agent/system_prompt.md

@@ -30,16 +30,13 @@ 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.** For multi-step work, call update_plan with an ordered checklist and
-   keep it live. The plan bar is the user's progress signal — call update_plan
-   after EACH concrete unit of work (every file written, every command run), not
-   just at coarse milestones: mark the finished item completed and the next one
-   in_progress before you start it. A plan stuck at 0/N while files are landing is
-   a bug, not economy — these calls are cheap, expected, and the update_plan cards
-   are hidden from the transcript, so frequent updates cost the user nothing and
-   never clutter the conversation. Keep at most one item in_progress, and never
-   batch the updates to the end of the turn. Skip the plan for trivial one-step
-   tasks.
+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.
 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
@@ -54,11 +51,16 @@ work.
 ## Editing discipline
 
 - Choose the narrowest tool that safely accomplishes the step. Prefer native
-  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.
+  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.
   They are safer, reviewable, and produce clean diffs.
-- Make one tool call per file. Do not batch multi-file writes into a single
-  shell or script invocation.
+- 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.
 - For edits to existing files, prefer edit_file or apply_patch with minimal,
   targeted diffs. Match the existing indentation, imports, and idioms. Match the
   file's comment density: do not add explanatory comments unless the user asks or

internal/agent/system_prompt_models.go

@@ -71,5 +71,6 @@ 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 for any multi-step task and keep it current.
+- 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.
 </model_guidance>`

internal/agent/tool_result_images_test.go

@@ -0,0 +1,207 @@
+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,8 +68,11 @@ 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
+	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
 	Redacted     bool
 	ChangedFiles []string
 	// ChangeSummaries are non-selectable generated-tree summaries emitted by
@@ -427,13 +430,6 @@ type Result struct {
 	IncompleteReason string
 }
 
-// Truncated reports whether the final response ended abnormally (cut off at the
-// output token cap or withheld by a content filter) rather than completing
-// naturally. Callers can use it to warn the user that FinalAnswer is incomplete.
-func (result Result) Truncated() bool {
-	return result.FinishReason != ""
-}
-
 // TruncationNotice returns a user-facing warning when the final response was
 // truncated, or "" for a normal completion. Shared by the CLI and TUI so the
 // wording stays consistent.

internal/agenteval/agent_command.go

@@ -31,10 +31,6 @@ type AgentRunner interface {
 
 type AgentRunnerFunc func(context.Context, AgentRunInput) AgentRunResult
 
-func (fn AgentRunnerFunc) Run(ctx context.Context, input AgentRunInput) AgentRunResult {
-	return fn(ctx, input)
-}
-
 // defaultAgentOutputLimit caps captured stdout/stderr per stream so a chatty or
 // runaway agent cannot exhaust memory or bloat the benchmark report.
 const defaultAgentOutputLimit = 1 << 20 // 1 MiB per stream

internal/agenteval/export_test.go

@@ -0,0 +1,8 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package agenteval
+
+import "context"
+
+func (fn AgentRunnerFunc) Run(ctx context.Context, input AgentRunInput) AgentRunResult {
+	return fn(ctx, input)
+}

internal/agenteval/score.go

@@ -125,9 +125,7 @@ func Score(suite Suite, input ScoreInput) Report {
 	if len(task.RequiredTraceEvents) > 0 {
 		report.Results = append(report.Results, scoreTraceEvents(task.RequiredTraceEvents, input))
 	}
-	for _, result := range unknownCommandResults(input.CommandResults, seenCommands, input) {
-		report.Results = append(report.Results, result)
-	}
+	report.Results = append(report.Results, unknownCommandResults(input.CommandResults, seenCommands, input)...)
 	report.finishSummary()
 	return report
 }

internal/agentinit/agentinit.go

@@ -40,14 +40,14 @@ func FormatFacts(info repoinfo.Info) string {
 	b.WriteString("Pre-computed repository facts (from a local scan — verify and fill gaps):\n")
 
 	if info.PrimaryLanguage != "" {
-		b.WriteString(fmt.Sprintf("- Primary language: %s (of %d detected)\n", info.PrimaryLanguage, info.LanguageCount))
+		fmt.Fprintf(&b, "- Primary language: %s (of %d detected)\n", info.PrimaryLanguage, info.LanguageCount)
 	}
 	if langs := topLanguages(info.Languages, 5); langs != "" {
 		b.WriteString("- Languages: " + langs + "\n")
 	}
-	b.WriteString(fmt.Sprintf("- Size: ~%d files, ~%d LOC, max dir depth %d\n", info.FileCount, info.LOCEstimate, info.MaxDepth))
+	fmt.Fprintf(&b, "- Size: ~%d files, ~%d LOC, max dir depth %d\n", info.FileCount, info.LOCEstimate, info.MaxDepth)
 	if info.WorkspaceType != "" && info.WorkspaceType != "none" {
-		b.WriteString(fmt.Sprintf("- Workspace: %s (%d packages)\n", info.WorkspaceType, info.WorkspacePackageCount))
+		fmt.Fprintf(&b, "- Workspace: %s (%d packages)\n", info.WorkspaceType, info.WorkspacePackageCount)
 	}
 	if len(info.BuildTools) > 0 {
 		b.WriteString("- Build tools: " + strings.Join(info.BuildTools, ", ") + "\n")

internal/background/process_posix_test.go

@@ -149,12 +149,9 @@ func TestTerminateProcessKillsForkedChildren(t *testing.T) {
 
 	// The forked child must be gone too (poll until reaped by init).
 	deadline := time.Now().Add(2 * time.Second)
-	for {
-		// Only ESRCH proves the child is gone; any other error (e.g. EPERM) would
-		// wrongly pass the test, so treat it as still-present and keep polling.
-		if errors.Is(syscall.Kill(childPID, syscall.Signal(0)), syscall.ESRCH) {
-			break // child no longer exists
-		}
+	// Only ESRCH proves the child is gone; any other error (e.g. EPERM) would
+	// wrongly pass the test, so treat it as still-present and keep polling.
+	for !errors.Is(syscall.Kill(childPID, syscall.Signal(0)), syscall.ESRCH) {
 		if time.Now().After(deadline) {
 			_ = syscall.Kill(childPID, syscall.SIGKILL)
 			t.Fatalf("forked child %d survived terminateProcess — group kill failed", childPID)

internal/cli/acp.go

@@ -27,6 +27,8 @@ Usage:
 Not meant to be run interactively — point your editor's ACP / external-agent
 setting at "zero acp".`
 
+var acpSignalContext = signalContext
+
 // runACP serves ACP over stdio so an editor can drive ZERO's agent core. It
 // speaks JSON-RPC 2.0 (newline-delimited JSON) on stdin/stdout; stderr stays free
 // for human-readable diagnostics. The session lifecycle maps onto ZERO's own
@@ -42,7 +44,9 @@ func runACP(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int
 		}
 	}
 
-	conn := acp.NewConn(deps.stdin, stdout)
+	// The process's own stdin belongs exclusively to this Conn for the life of
+	// the command, so cancellation may close it to interrupt an idle read.
+	conn := acp.NewOwnedConn(deps.stdin, stdout)
 	acp.NewAgent(conn, acp.Deps{
 		ResolveConfig: deps.resolveConfig,
 		DiscoverModels: func(ctx context.Context, profile config.ProviderProfile) ([]providermodeldiscovery.Model, error) {
@@ -73,9 +77,9 @@ func runACP(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int
 		AgentInfo:            acp.Implementation{Name: "zero", Version: version},
 	})
 
-	ctx, stop := signalContext()
+	ctx, stop := acpSignalContext()
 	defer stop()
-	if err := conn.Serve(ctx); err != nil && ctx.Err() == nil {
+	if err := conn.Serve(ctx); err != nil {
 		return writeAppError(stderr, "acp: "+err.Error(), exitCrash)
 	}
 	return exitSuccess

internal/cli/acp_test.go

@@ -0,0 +1,120 @@
+package cli
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"io"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+)
+
+type acpTestReader func([]byte) (int, error)
+
+func (r acpTestReader) Read(p []byte) (int, error) { return r(p) }
+
+type acpTestWriter func([]byte) (int, error)
+
+func (w acpTestWriter) Write(p []byte) (int, error) { return w(p) }
+
+func installACPTestContext(t *testing.T) context.CancelFunc {
+	t.Helper()
+	ctx, cancel := context.WithCancel(context.Background())
+	previous := acpSignalContext
+	acpSignalContext = func() (context.Context, context.CancelFunc) {
+		return ctx, cancel
+	}
+	t.Cleanup(func() {
+		cancel()
+		acpSignalContext = previous
+	})
+	return cancel
+}
+
+func TestRunACPCancellationPreservesTerminalReadError(t *testing.T) {
+	cancel := installACPTestContext(t)
+	wantErr := errors.New("transport read failed")
+	reader := acpTestReader(func(p []byte) (int, error) {
+		return copy(p, `{"jsonrpc":"1.0","id":1}`+"\n"), wantErr
+	})
+	writeStarted := make(chan struct{}, 1)
+	releaseWrite := make(chan struct{})
+	var stdout bytes.Buffer
+	writer := acpTestWriter(func(p []byte) (int, error) {
+		writeStarted <- struct{}{}
+		<-releaseWrite
+		return stdout.Write(p)
+	})
+	var stderr bytes.Buffer
+	done := make(chan int, 1)
+	go func() {
+		done <- runACP(nil, writer, &stderr, fillAppDeps(appDeps{stdin: reader}))
+	}()
+
+	select {
+	case <-writeStarted:
+	case <-time.After(time.Second):
+		t.Fatal("ACP did not reach the response write")
+	}
+	// Keep Serve in the synchronous write until cancellation is observable. The
+	// regression requires the genuine read error to win over that cancellation.
+	cancel()
+	close(releaseWrite)
+
+	select {
+	case code := <-done:
+		if code != exitCrash {
+			t.Fatalf("exit code = %d, want crash %d", code, exitCrash)
+		}
+	case <-time.After(time.Second):
+		t.Fatal("ACP did not exit after terminal read error")
+	}
+	if got := stderr.String(); !strings.Contains(got, "acp: "+wantErr.Error()) {
+		t.Fatalf("stderr = %q, want terminal read error", got)
+	}
+}
+
+func TestRunACPIdleCancellationExitsCleanly(t *testing.T) {
+	cancel := installACPTestContext(t)
+	pipeReader, pipeWriter := io.Pipe()
+	t.Cleanup(func() { _ = pipeWriter.Close() })
+	readStarted := make(chan struct{}, 1)
+	reader := &acpNotifyingReadCloser{ReadCloser: pipeReader, readStarted: readStarted}
+	var stdout, stderr bytes.Buffer
+	done := make(chan int, 1)
+	go func() {
+		done <- runACP(nil, &stdout, &stderr, fillAppDeps(appDeps{stdin: reader}))
+	}()
+
+	select {
+	case <-readStarted:
+	case <-time.After(time.Second):
+		t.Fatal("ACP did not begin reading")
+	}
+	cancel()
+
+	select {
+	case code := <-done:
+		if code != exitSuccess {
+			t.Fatalf("exit code = %d, want success %d; stderr: %s", code, exitSuccess, stderr.String())
+		}
+	case <-time.After(time.Second):
+		t.Fatal("ACP did not exit after cancellation")
+	}
+	if stderr.Len() != 0 {
+		t.Fatalf("stderr = %q, want empty", stderr.String())
+	}
+}
+
+type acpNotifyingReadCloser struct {
+	io.ReadCloser
+	readStarted chan<- struct{}
+	once        sync.Once
+}
+
+func (r *acpNotifyingReadCloser) Read(p []byte) (int, error) {
+	r.once.Do(func() { r.readStarted <- struct{}{} })
+	return r.ReadCloser.Read(p)
+}

internal/cli/agent_eval.go

@@ -515,9 +515,7 @@ func agentEvalReportFromBenchmark(suite agenteval.Suite, report agenteval.Benchm
 		if task.Agent.Truncated {
 			converted.Truncated = true
 		}
-		for _, failure := range agentEvalFailuresFromTaskReport(task) {
-			converted.Failures = append(converted.Failures, failure)
-		}
+		converted.Failures = append(converted.Failures, agentEvalFailuresFromTaskReport(task)...)
 	}
 	return converted
 }

internal/cli/app.go

@@ -585,6 +585,9 @@ func fillAppDeps(deps appDeps) appDeps {
 	baseProbeProviderHealth := deps.probeProviderHealth
 	deps.probeProviderHealth = func(ctx context.Context, options providerhealth.Options) providerhealth.Result {
 		options.Profile = applyStoredProviderKeyAt(options.Profile, userConfigPath)
+		if options.OAuthResolver == nil {
+			options.OAuthResolver, _ = oauthLoginForProfile(options.Profile)
+		}
 		return baseProbeProviderHealth(ctx, options)
 	}
 	return deps

internal/cli/completions.go

@@ -59,7 +59,7 @@ var completionRoot = completionNode{
 			{names: []string{"delete", "rm"}}, {names: []string{"edit"}}, {names: []string{"path"}},
 		}},
 		{names: []string{"plugins", "plugin"}, children: []completionNode{
-			{names: []string{"list"}}, {names: []string{"add"}}, {names: []string{"remove", "rm"}},
+			{names: []string{"list"}}, {names: []string{"add"}}, {names: []string{"info"}}, {names: []string{"remove", "rm"}},
 		}},
 		{names: []string{"backends", "backend"}, children: leafNodes("doctor")},
 		{names: []string{"skills", "skill"}, children: []completionNode{

internal/cli/completions_test.go

@@ -159,6 +159,8 @@ func TestCompletionTreeCoversAliasesNestingAndCommonFlags(t *testing.T) {
 	assertCandidates(t, byPath["mcp oauth"], "login", "logout", "status")
 	assertCandidates(t, byPath["sandbox grants"], "list", "allow", "deny", "revoke", "clear")
 	assertCandidates(t, byPath["completions"], "bash", "zsh", "fish", "powershell", "elvish")
+	assertCandidates(t, byPath["plugins"], "list", "add", "info", "remove", "rm")
+	assertCandidates(t, byPath["plugin"], "list", "add", "info", "remove", "rm")
 }
 
 func assertCandidates(t *testing.T, got []string, wants ...string) {

internal/cli/context.go

@@ -40,10 +40,11 @@ func runContext(args []string, stdout io.Writer, stderr io.Writer, deps appDeps)
 		return writeAppError(stderr, err.Error(), exitCrash)
 	}
 	report, err := contextreport.Build(contextreport.Options{
-		WorkspaceRoot: workspaceRoot,
-		Provider:      resolved.Provider,
-		Registry:      newCoreRegistry(workspaceRoot),
-		ContextWindow: modelContextWindow(modelRegistry, resolved.Provider.Model),
+		WorkspaceRoot:  workspaceRoot,
+		Provider:       resolved.Provider,
+		Registry:       newCoreRegistry(workspaceRoot),
+		ContextWindow:  modelContextWindow(modelRegistry, resolved.Provider.Model),
+		DeferThreshold: resolved.Tools.DeferThreshold,
 	})
 	if err != nil {
 		return writeAppError(stderr, err.Error(), exitCrash)

internal/cli/deferred_wiring_test.go

@@ -77,11 +77,10 @@ func TestRegisterToolSearchIfEligibleSkipsWhenThresholdZero(t *testing.T) {
 	}
 }
 
-func TestDeferredEligibleCountIgnoresCoreTools(t *testing.T) {
+func TestDeferredEligibleCountIncludesOptionalBuiltins(t *testing.T) {
 	registry := newCoreRegistry(t.TempDir())
-	// newCoreRegistry holds only built-ins; none implement Deferred().
-	if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got != 0 {
-		t.Fatalf("deferredEligibleCount(core) = %d, want 0", got)
+	if got := deferredEligibleCount(registry, agent.PermissionModeAuto, nil, nil); got < 2 {
+		t.Fatalf("deferredEligibleCount(core) = %d, want at least 2 optional built-ins", got)
 	}
 }
 

internal/cli/distribution.go

@@ -139,6 +139,7 @@ func runSkillInfo(args []string, dir string, stdout io.Writer, stderr io.Writer)
 	}
 	if info.Hash != "" {
 		lines = append(lines, "  hash: "+info.Hash)
+		lines = append(lines, fmt.Sprintf("  hash drift: %t", info.HashDrift))
 	}
 	lines = append(lines, "  path: "+info.Skill.Path)
 	if _, err := fmt.Fprintln(stdout, redaction.RedactString(strings.Join(lines, "\n"), redaction.Options{})); err != nil {

internal/cli/distribution_test.go

@@ -83,6 +83,48 @@ func TestRunSkillAddInfoRemove(t *testing.T) {
 	if !strings.Contains(info, src) || !strings.Contains(info, "sha256:") {
 		t.Fatalf("info missing source/hash:\n%s", info)
 	}
+	if !strings.Contains(info, "hash drift: false") {
+		t.Fatalf("info missing hash drift status:\n%s", info)
+	}
+
+	stdout.Reset()
+	stderr.Reset()
+	if exit := runWithDeps([]string{"skill", "info", "confirmation-policy", "--json"}, &stdout, &stderr, deps); exit != 0 {
+		t.Fatalf("skill info --json exit = %d, stderr = %s", exit, stderr.String())
+	}
+	var beforeJSON map[string]any
+	if err := json.Unmarshal(stdout.Bytes(), &beforeJSON); err != nil {
+		t.Fatalf("parse skill info json: %v\n%s", err, stdout.String())
+	}
+	if beforeJSON["hashDrift"] != false {
+		t.Fatalf("hashDrift before edit = %#v, want false", beforeJSON["hashDrift"])
+	}
+
+	skillPath := filepath.Join(skillsDir, "confirmation-policy", "SKILL.md")
+	if err := os.WriteFile(skillPath, []byte("---\ndescription: edited\n---\nchanged\n"), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	stdout.Reset()
+	stderr.Reset()
+	if exit := runWithDeps([]string{"skill", "info", "confirmation-policy"}, &stdout, &stderr, deps); exit != 0 {
+		t.Fatalf("skill info after edit exit = %d, stderr = %s", exit, stderr.String())
+	}
+	if !strings.Contains(stdout.String(), "hash drift: true") {
+		t.Fatalf("info missing hash drift after edit:\n%s", stdout.String())
+	}
+
+	stdout.Reset()
+	stderr.Reset()
+	if exit := runWithDeps([]string{"skill", "info", "confirmation-policy", "--json"}, &stdout, &stderr, deps); exit != 0 {
+		t.Fatalf("skill info --json after edit exit = %d, stderr = %s", exit, stderr.String())
+	}
+	var afterJSON map[string]any
+	if err := json.Unmarshal(stdout.Bytes(), &afterJSON); err != nil {
+		t.Fatalf("parse skill info json after edit: %v\n%s", err, stdout.String())
+	}
+	if afterJSON["hashDrift"] != true {
+		t.Fatalf("hashDrift after edit = %#v, want true", afterJSON["hashDrift"])
+	}
 
 	// remove deletes it
 	stdout.Reset()

internal/cli/exec_turn_session_test.go

@@ -5,6 +5,7 @@ import (
 	"net/http"
 	"net/http/httptest"
 	"strings"
+	"sync"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -24,12 +25,47 @@ func TestRunExecOptimizedSessionUnderGate(t *testing.T) {
 	cwd := t.TempDir()
 
 	var heads, posts atomic.Int64
+	// Closed when the prewarm probe lands, so the turn can be held open until it
+	// does. See the POST handler for why that ordering has to be forced.
+	headSeen := make(chan struct{})
+	var headOnce sync.Once
+	var prewarmWaitTimedOut atomic.Bool
 	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		switch {
 		case r.Method == http.MethodHead:
 			heads.Add(1)
+			headOnce.Do(func() { close(headSeen) })
 			w.WriteHeader(http.StatusMethodNotAllowed)
 		case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/chat/completions"):
+			// Hold the turn open until the prewarm probe arrives. The probe is
+			// issued on the run's context, and runExec cancels that context the
+			// moment it returns, because the stop function signalContext hands
+			// back is a CancelFunc. A probe that has not reached the wire by the
+			// end of the run therefore never reaches it at all. That is why the
+			// poll this replaced could not work at any deadline: once the run is
+			// over the probe is cancelled, not merely slow.
+			//
+			// This widens the window, it does not close it. Once the goroutine is
+			// scheduled it still has only prewarmTimeout to dial loopback and
+			// write the request, and that timeout is armed inside the goroutine
+			// itself, so holding the turn here cannot extend it. If this test
+			// ever fails again, prewarmTimeout is where to look, not this hold.
+			//
+			// No self-deadlock: the pinned transport leaves MaxConnsPerHost at 0,
+			// so the HEAD dials its own connection instead of queueing behind
+			// this held POST. Do not set a per-host connection limit there.
+			//
+			// Bounded, so a probe that genuinely never fires fails the assertion
+			// below with a useful message instead of hanging the test.
+			select {
+			case <-headSeen:
+			case <-time.After(10 * time.Second):
+				// Record it. A probe that shows up after this gave up would still
+				// leave heads at 1, so the count alone proves the probe was
+				// eventually received, not that it was ordered before the turn,
+				// which is the whole claim of this test.
+				prewarmWaitTimedOut.Store(true)
+			}
 			posts.Add(1)
 			w.Header().Set("Content-Type", "text/event-stream")
 			_, _ = w.Write([]byte("data: {\"choices\":[{\"delta\":{\"content\":\"done\"}}]}\n\n"))
@@ -75,14 +111,13 @@ func TestRunExecOptimizedSessionUnderGate(t *testing.T) {
 	if got := posts.Load(); got < 1 {
 		t.Fatalf("server saw %d chat-completions POSTs, want >= 1", got)
 	}
-	// The prewarm probe is asynchronous; it fires well before the model turn
-	// finishes, but poll rather than assuming scheduling order. The deadline
-	// exceeds the production prewarmTimeout (3s) so a slow probe cannot flake
-	// this assertion.
-	deadline := time.Now().Add(5 * time.Second)
-	for heads.Load() == 0 && time.Now().Before(deadline) {
-		time.Sleep(10 * time.Millisecond)
+	// Checked before the count, because the count cannot tell the two apart: a
+	// probe that arrived only after the guard expired still leaves heads at 1.
+	if prewarmWaitTimedOut.Load() {
+		t.Fatal("the turn was not held until the prewarm probe arrived; the probe did not precede it")
 	}
+	// No polling: the turn above could not complete until the probe landed, so
+	// the count is settled by this point.
 	if got := heads.Load(); got != 1 {
 		t.Fatalf("server saw %d prewarm HEAD probes, want exactly 1", got)
 	}

internal/cli/mcp_config.go

@@ -308,6 +308,34 @@ 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 {
@@ -734,14 +762,12 @@ func (cfg *mcpWritableConfig) setServerDisabled(name string, disabled bool) (boo
 		switch {
 		case legacyFound:
 			raw = legacyRaw
-			found = true
 		case config.IsDefaultMCPServer(name):
 			// A built-in default server isn't written to the file until the user
 			// overrides it. Treat it as present with an empty base so disabling it
 			// writes a minimal {"disabled":true} entry that merges over the default —
 			// letting `zero mcp disable <default>` work even though it lives in code.
 			raw = nil
-			found = true
 		default:
 			return false, false, nil
 		}

internal/cli/mcp_trust_edge_test.go

@@ -6,6 +6,7 @@ package cli
 
 import (
 	"bytes"
+	"encoding/json"
 	"errors"
 	"os"
 	"path/filepath"
@@ -219,3 +220,81 @@ 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/cli/observability_test.go

@@ -5,6 +5,8 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
@@ -12,6 +14,7 @@ import (
 	"time"
 
 	"github.com/Gitlawb/zero/internal/config"
+	"github.com/Gitlawb/zero/internal/oauth"
 	"github.com/Gitlawb/zero/internal/providerhealth"
 	"github.com/Gitlawb/zero/internal/sessions"
 )
@@ -109,6 +112,65 @@ func TestRunDoctorConnectivityProbesProvider(t *testing.T) {
 	}
 }
 
+func TestRunDoctorConnectivityUsesOAuthLogin(t *testing.T) {
+	tokenPath := filepath.Join(t.TempDir(), "oauth-tokens.json")
+	t.Setenv("XDG_CONFIG_HOME", t.TempDir())
+	t.Setenv("ZERO_OAUTH_STORAGE", "file")
+	t.Setenv("ZERO_OAUTH_TOKENS_PATH", tokenPath)
+	store, err := oauth.NewStore(oauth.StoreOptions{FilePath: tokenPath})
+	if err != nil {
+		t.Fatalf("NewStore: %v", err)
+	}
+	if err := store.Save(oauth.ProviderKey("xai"), oauth.Token{
+		AccessToken: "oauth-doctor-token",
+		ExpiresAt:   time.Now().Add(time.Hour),
+	}); err != nil {
+		t.Fatalf("seed OAuth token: %v", err)
+	}
+
+	var gotAuth string
+	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		gotAuth = r.Header.Get("Authorization")
+		w.Header().Set("Content-Type", "application/json")
+		_, _ = w.Write([]byte(`{"data":[]}`))
+	}))
+	defer server.Close()
+
+	profile := config.ProviderProfile{
+		Name:         "xai",
+		CatalogID:    "xai",
+		ProviderKind: config.ProviderKindOpenAICompatible,
+		BaseURL:      server.URL + "/v1",
+		Model:        "grok-4",
+	}
+	var stdout bytes.Buffer
+	var stderr bytes.Buffer
+	exitCode := runWithDeps([]string{"doctor", "--connectivity"}, &stdout, &stderr, appDeps{
+		getwd: func() (string, error) {
+			return t.TempDir(), nil
+		},
+		resolveConfig: func(string, config.Overrides) (config.ResolvedConfig, error) {
+			return config.ResolvedConfig{
+				Provider:       profile,
+				Providers:      []config.ProviderProfile{profile},
+				ActiveProvider: profile.Name,
+			}, nil
+		},
+		probeProviderHealth: providerhealth.Probe,
+		now:                 fixedCLITime("2026-07-29T20:00:00Z"),
+	})
+
+	if exitCode != exitSuccess {
+		t.Fatalf("doctor exit = %d, want success; stderr=%q\n%s", exitCode, stderr.String(), stdout.String())
+	}
+	if gotAuth != "Bearer oauth-doctor-token" {
+		t.Fatalf("Authorization = %q, want OAuth bearer", gotAuth)
+	}
+	if !strings.Contains(stdout.String(), "[pass] provider.connectivity") {
+		t.Fatalf("doctor did not report passing connectivity:\n%s", stdout.String())
+	}
+}
+
 func TestRunDoctorConnectivityReportsProviderHealthFailure(t *testing.T) {
 	var stdout bytes.Buffer
 	var stderr bytes.Buffer

internal/cli/provider_catalog_onboarding_test.go

@@ -19,6 +19,7 @@ func TestProviderCatalogRuntimeProvidersShowOnboardingSetupHints(t *testing.T) {
 		{id: "openai", name: "OpenAI", transport: "openai", defaultModel: "gpt-4.1"},
 		{id: "groq", name: "Groq", transport: "openai-compatible", defaultModel: "llama-3.3-70b-versatile"},
 		{id: "longcat", name: "LongCat", transport: "openai-compatible", defaultModel: "LongCat-2.0"},
+		{id: "fireworks", name: "Fireworks AI", transport: "openai-compatible", defaultModel: "accounts/fireworks/models/kimi-k2p7-code"},
 		{id: "ollama-cloud", name: "Ollama Cloud", transport: "openai-compatible", defaultModel: "qwen3-coder:480b"},
 		{id: "ollama", name: "Ollama Local", transport: "openai-compatible", defaultModel: "llama3.1"},
 	}

internal/cli/provider_detect.go

@@ -83,10 +83,10 @@ func runProvidersDetect(args []string, stdout io.Writer, stderr io.Writer, deps
 func parseProviderDetectArgs(args []string) (providerDetectOptions, bool, error) {
 	options := providerDetectOptions{}
 	for _, arg := range args {
-		switch {
-		case arg == "-h" || arg == "--help" || arg == "help":
+		switch arg {
+		case "-h", "--help", "help":
 			return options, true, nil
-		case arg == "--json":
+		case "--json":
 			options.json = true
 		default:
 			return options, false, execUsageError{fmt.Sprintf("unexpected argument %q", arg)}

internal/cli/usage.go

@@ -244,19 +244,19 @@ func parseUsageArgs(args []string) (usageOptions, bool, error) {
 func FormatReport(report usage.Report, insertions int, deletions int) string {
 	var builder strings.Builder
 	builder.WriteString("Usage report (cost is a reconstructed estimate)\n\n")
-	builder.WriteString(fmt.Sprintf("%-12s %10s %14s %14s\n", "date", "requests", "tokens", "est. cost"))
+	fmt.Fprintf(&builder, "%-12s %10s %14s %14s\n", "date", "requests", "tokens", "est. cost")
 	for _, bucket := range report.Buckets {
-		builder.WriteString(fmt.Sprintf("%-12s %10d %14s %14s\n",
-			bucket.Date, bucket.Requests, groupThousands(bucket.TotalTokens), formatUSD(bucket.TotalCost)))
+		fmt.Fprintf(&builder, "%-12s %10d %14s %14s\n",
+			bucket.Date, bucket.Requests, groupThousands(bucket.TotalTokens), formatUSD(bucket.TotalCost))
 	}
-	builder.WriteString(fmt.Sprintf("\n%-12s %10d %14s %14s\n",
-		"total", report.Total.Requests, groupThousands(report.Total.TotalTokens), formatUSD(report.Total.TotalCost)))
+	fmt.Fprintf(&builder, "\n%-12s %10d %14s %14s\n",
+		"total", report.Total.Requests, groupThousands(report.Total.TotalTokens), formatUSD(report.Total.TotalCost))
 
-	builder.WriteString(fmt.Sprintf("\nnet LOC (estimate): +%d / -%d = %d\n",
-		insertions, deletions, report.NetLOC))
+	fmt.Fprintf(&builder, "\nnet LOC (estimate): +%d / -%d = %d\n",
+		insertions, deletions, report.NetLOC)
 	if report.NetLOCPositive {
-		builder.WriteString(fmt.Sprintf("tokens per net LOC: %.1f\n", report.TokensPerNetLOC))
-		builder.WriteString(fmt.Sprintf("est. cost per net LOC: %s\n", formatUSD(report.CostPerNetLOC)))
+		fmt.Fprintf(&builder, "tokens per net LOC: %.1f\n", report.TokensPerNetLOC)
+		fmt.Fprintf(&builder, "est. cost per net LOC: %s\n", formatUSD(report.CostPerNetLOC))
 	} else {
 		builder.WriteString("tokens per net LOC: n/a (net LOC <= 0)\n")
 		builder.WriteString("est. cost per net LOC: n/a (net LOC <= 0)\n")

internal/config/command.go

@@ -8,6 +8,7 @@ import (
 	"os/exec"
 	"runtime"
 	"strings"
+	"sync"
 	"time"
 )
 
@@ -17,7 +18,11 @@ func LoadProviderCommand(command string) (FileConfig, error) {
 	stdout, stderr, err := runProviderCommand(command, providerCommandTimeout)
 	if err != nil {
 		if errors.Is(err, errProviderCommandTimeout) {
-			return FileConfig{}, fmt.Errorf("provider command timed out after 5s")
+			// Report which phase ran out of time, and anything the command
+			// managed to say. This used to be a bare string with the underlying
+			// error and stderr both dropped, which left a user whose command was
+			// slow on a cold machine with nothing to act on.
+			return FileConfig{}, fmt.Errorf("provider command timed out after %s: %w%s", providerCommandTimeout, err, commandOutput(stderr))
 		}
 		return FileConfig{}, fmt.Errorf("provider command failed: %w%s", err, commandOutput(stderr))
 	}
@@ -40,34 +45,150 @@ func LoadProviderCommand(command string) (FileConfig, error) {
 
 var errProviderCommandTimeout = errors.New("provider command timeout")
 
+// providerCommandDrainGrace bounds how long the timeout path waits for cmd.Wait
+// to return after the process tree has been terminated. Terminate normally makes
+// that immediate; this exists so a descendant that refuses to die cannot extend
+// a call whose deadline has already expired.
+const providerCommandDrainGrace = 2 * time.Second
+
+// syncBuffer serialises the command's I/O pump against reads from this package.
+// On the timeout path runProviderCommand can return while cmd.Wait is still
+// draining, so the buffers outlive the call and a plain bytes.Buffer would be
+// read and written concurrently.
+type syncBuffer struct {
+	mu  sync.Mutex
+	buf bytes.Buffer
+}
+
+func (b *syncBuffer) Write(p []byte) (int, error) {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return b.buf.Write(p)
+}
+
+// snapshot copies out what has been written so far. A copy rather than the
+// buffer's own slice, which the pump may still append to.
+func (b *syncBuffer) snapshot() []byte {
+	b.mu.Lock()
+	defer b.mu.Unlock()
+	return append([]byte(nil), b.buf.Bytes()...)
+}
+
+// awaitWithGrace waits for done and gives up after grace, reporting whether the
+// wait completed rather than expired.
+//
+// Split out from the timeout path so the bound can be asserted on its own. In
+// place it is effectively untestable: Terminate kills the tree, so Wait returns
+// promptly and an unbounded receive would behave identically in every test that
+// is not pathological, which is exactly how the old unbounded receive survived.
+func awaitWithGrace(done <-chan error, grace time.Duration) bool {
+	timer := time.NewTimer(grace)
+	defer timer.Stop()
+	select {
+	case <-done:
+		return true
+	case <-timer.C:
+		return false
+	}
+}
+
+// runProviderCommand runs a provider command under a real deadline.
+//
+// "Real" is the point: timeout used to bound only the wait, so the call could
+// take arbitrarily longer than it. Two phases sat outside it. Starting the
+// process ran before the clock began, and on Windows that is CREATE_SUSPENDED,
+// job-object creation and assignment, and a system-wide thread snapshot, none of
+// which is fast on a cold or contended machine. Draining after termination was
+// unbounded as well. Both are inside the budget now, so the deadline is an upper
+// bound rather than a floor.
 func runProviderCommand(command string, timeout time.Duration) ([]byte, []byte, error) {
 	cmd := shellCommand(command)
-	configureCommandProcess(cmd)
+	// Bound the time Wait spends draining I/O pipes after the process exits,
+	// in case an orphaned descendant still holds the write ends open.
+	cmd.WaitDelay = time.Second
 
-	var stdout bytes.Buffer
-	var stderr bytes.Buffer
-	cmd.Stdout = &stdout
-	cmd.Stderr = &stderr
+	stdout := &syncBuffer{}
+	stderr := &syncBuffer{}
+	cmd.Stdout = stdout
+	cmd.Stderr = stderr
 
-	if err := cmd.Start(); err != nil {
-		return nil, nil, err
+	// Armed before the process is started, so process creation is inside the
+	// budget rather than ahead of it.
+	deadline := time.NewTimer(timeout)
+	defer deadline.Stop()
+
+	type startResult struct {
+		proc *commandProcess
+		err  error
 	}
+	// Buffered, so the goroutine can finish and exit even when nobody is left
+	// waiting on it because the deadline already fired.
+	started := make(chan startResult, 1)
+	go func() {
+		proc, err := startCommandProcess(cmd)
+		started <- startResult{proc: proc, err: err}
+	}()
+
+	var proc *commandProcess
+	select {
+	case result := <-started:
+		if result.err != nil {
+			return stdout.snapshot(), stderr.snapshot(), result.err
+		}
+		proc = result.proc
+	case <-deadline.C:
+		// The start is still in flight and will eventually hand back whatever it
+		// created, so cleanup is handed off rather than abandoned. Wait reaps the
+		// process; without it a started-but-orphaned command becomes a zombie.
+		go func() {
+			result := <-started
+			if result.err != nil {
+				return
+			}
+			result.proc.Terminate()
+			_ = cmd.Wait()
+			result.proc.Close()
+		}()
+		return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: starting the command exceeded %s", errProviderCommandTimeout, timeout)
+	}
+	defer proc.Close()
 
 	done := make(chan error, 1)
 	go func() {
 		done <- cmd.Wait()
 	}()
 
-	timer := time.NewTimer(timeout)
-	defer timer.Stop()
-
 	select {
 	case err := <-done:
-		return stdout.Bytes(), stderr.Bytes(), err
-	case <-timer.C:
-		terminateCommandProcess(cmd)
-		<-done
-		return stdout.Bytes(), stderr.Bytes(), errProviderCommandTimeout
+		if err != nil {
+			// The shell exited (whether via ErrWaitDelay because a
+			// background descendant kept the inherited stdout/stderr pipes
+			// open, or via a nonzero exit status) but a leftover descendant
+			// may still be running. Terminate is a no-op against an
+			// already-dead tree, so always call it rather than gating on
+			// the specific error to avoid leaking that descendant.
+			proc.Terminate()
+		}
+		if errors.Is(err, exec.ErrWaitDelay) {
+			// Wrap rather than replace. A descendant holding the inherited pipes
+			// open and the command never finishing at all are different failures
+			// that both collapsed into the same bare sentinel, which left callers
+			// and tests unable to tell which one happened. Callers that only ask
+			// errors.Is(err, errProviderCommandTimeout) are unaffected, so the
+			// message LoadProviderCommand reports is unchanged.
+			return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: %w", errProviderCommandTimeout, err)
+		}
+		return stdout.snapshot(), stderr.snapshot(), err
+	case <-deadline.C:
+		proc.Terminate()
+		// Bounded, unlike the bare receive this replaces. Terminate kills the
+		// tree so Wait almost always returns at once, but "almost always" was
+		// doing real work here: a tree that resists termination used to hold the
+		// call open indefinitely, long past the deadline that had just expired.
+		// Returning early leaves cmd.Wait running, which is why the buffers are
+		// synchronised.
+		awaitWithGrace(done, providerCommandDrainGrace)
+		return stdout.snapshot(), stderr.snapshot(), fmt.Errorf("%w: the command did not finish within %s", errProviderCommandTimeout, timeout)
 	}
 }
 

internal/config/command_test.go

@@ -1,9 +1,12 @@
 package config
 
 import (
+	"errors"
 	"os"
+	"os/exec"
 	"path/filepath"
 	"runtime"
+	"strconv"
 	"strings"
 	"testing"
 	"time"
@@ -67,7 +70,8 @@ func TestLoadProviderCommandFailureIncludesExitAndRedactsOutput(t *testing.T) {
 }
 
 func TestLoadProviderCommandTimeout(t *testing.T) {
-	command := writeCommand(t, commandScript{SleepSeconds: 10})
+	pidFile := filepath.Join(t.TempDir(), "sleep.pid")
+	command := writeCommand(t, commandScript{SleepSeconds: 10, PidFile: pidFile})
 
 	start := time.Now()
 	_, err := LoadProviderCommand(command)
@@ -85,6 +89,175 @@ func TestLoadProviderCommandTimeout(t *testing.T) {
 	if elapsed > maxElapsed {
 		t.Fatalf("timeout returned after %s, want roughly 5s", elapsed)
 	}
+	assertProcessTerminatedIfStarted(t, pidFile)
+}
+
+// providerCommandTestBudget bounds the two background-child tests below, and is
+// deliberately NOT providerCommandTimeout. Those tests assert which error each
+// cmd.Wait path yields and that the leftover descendant is killed; neither is a
+// claim about how fast Windows can start a process. Driving them through
+// LoadProviderCommand billed the fixture's own startup to the production 5s
+// budget, so on a loaded runner the outer timer won the select in
+// runProviderCommand: the nonzero-exit test saw a timeout instead of the exit
+// status it asserts, and the timer's Terminate killed the background child
+// before it recorded its PID, putting the PID file permanently out of reach no
+// matter how long the assertion polled for it.
+//
+// This is a hang-breaker, not a tuned margin. Nothing in a healthy run waits on
+// it, since both tests return about one WaitDelay after the fixture exits.
+const providerCommandTestBudget = 60 * time.Second
+
+// backgroundChildLifetime must exceed providerCommandTestBudget so that a child
+// dying of old age is unobservable: any run where runProviderCommand has not
+// returned yet trips the budget first. That is what lets both tests drop their
+// old elapsed-versus-lifetime bounds. "Still alive when the call returned" is
+// now true by construction, so a child found dead afterwards can only have been
+// killed by proc.Terminate.
+const backgroundChildLifetime = 120 * time.Second
+
+// TestLoadProviderCommandTerminatesBackgroundChild covers a command that
+// exits immediately but leaves a detached child holding the inherited
+// stdout/stderr pipes open (e.g. `sleep 600 & exit`). cmd.Wait() only
+// unblocks once WaitDelay elapses, and that path must still terminate the
+// leftover child instead of just returning and leaking it.
+func TestLoadProviderCommandTerminatesBackgroundChild(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "bg.pid")
+	command := writeCommand(t, commandScript{
+		Stdout:                 `{"name":"cmd","provider":"openai","apiKey":"sk-command","model":"gpt-command"}`,
+		BackgroundSleepSeconds: int(backgroundChildLifetime / time.Second),
+		BackgroundPidFile:      pidFile,
+	})
+
+	_, _, err := runProviderCommand(command, providerCommandTestBudget)
+	if err == nil {
+		t.Fatal("runProviderCommand() error = nil, want error from WaitDelay-bounded wait")
+	}
+	// Name the exact path. The outer-timer path reports the bare sentinel, so
+	// matching only that, or the "timed out after 5s" text this test used to
+	// match, would also be satisfied by a slow runner taking precisely the path
+	// this test exists to rule out.
+	if !errors.Is(err, errProviderCommandTimeout) || !errors.Is(err, exec.ErrWaitDelay) {
+		t.Fatalf("error = %v, want errProviderCommandTimeout wrapping exec.ErrWaitDelay", err)
+	}
+	assertProcessTerminated(t, pidFile)
+}
+
+// TestLoadProviderCommandTerminatesBackgroundChildOnFailure covers the same
+// leaked-descendant scenario as TestLoadProviderCommandTerminatesBackgroundChild,
+// but with a shell that exits nonzero. Go's exec.Cmd.Wait only returns
+// exec.ErrWaitDelay when the command itself exited successfully; a nonzero
+// exit yields the bare *ExitError instead, so the leftover child must still
+// be terminated on that path.
+func TestLoadProviderCommandTerminatesBackgroundChildOnFailure(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "bg-fail.pid")
+	command := writeCommand(t, commandScript{
+		Stderr:                 "boom",
+		ExitCode:               7,
+		BackgroundSleepSeconds: int(backgroundChildLifetime / time.Second),
+		BackgroundPidFile:      pidFile,
+	})
+
+	_, _, err := runProviderCommand(command, providerCommandTestBudget)
+	// The shell's own exit status has to survive. A timeout here means the
+	// budget swallowed it, which is exactly what this test kept reporting on CI.
+	if errors.Is(err, errProviderCommandTimeout) {
+		t.Fatalf("error = %v, want the shell's exit status rather than a timeout", err)
+	}
+	var exitErr *exec.ExitError
+	if !errors.As(err, &exitErr) {
+		t.Fatalf("error = %v, want *exec.ExitError from the nonzero exit", err)
+	}
+	if exitErr.ExitCode() != 7 {
+		t.Fatalf("exit code = %d, want 7", exitErr.ExitCode())
+	}
+	assertProcessTerminated(t, pidFile)
+}
+
+func TestAssertProcessTerminatedWaitsForDelayedPIDFile(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "delayed.pid")
+	firstMissing := make(chan struct{})
+	writeDone := make(chan error, 1)
+	go func() {
+		<-firstMissing
+		tempFile := pidFile + ".tmp"
+		if err := os.WriteFile(tempFile, []byte("2147483647\n"), 0o600); err != nil {
+			writeDone <- err
+			return
+		}
+		writeDone <- os.Rename(tempFile, pidFile)
+	}()
+
+	missingObserved := false
+	assertProcessTerminatedAfterMissing(t, pidFile, func() {
+		missingObserved = true
+		close(firstMissing)
+	})
+	if !missingObserved {
+		t.Fatal("PID file existed before the missing-file polling path was exercised")
+	}
+	if err := <-writeDone; err != nil {
+		t.Fatalf("write delayed pid file: %v", err)
+	}
+}
+
+func assertProcessTerminatedIfStarted(t *testing.T, pidFile string) {
+	t.Helper()
+
+	// The basic timeout fixture is intentionally unsynchronized. On a loaded
+	// Windows runner, the cold PowerShell child can be terminated before it
+	// records its PID; the synchronized background-child tests below retain
+	// the strict PID and liveness checks.
+	if _, err := os.Stat(pidFile); errors.Is(err, os.ErrNotExist) {
+		return
+	}
+	assertProcessTerminated(t, pidFile)
+}
+
+func assertProcessTerminated(t *testing.T, pidFile string) {
+	t.Helper()
+
+	assertProcessTerminatedAfterMissing(t, pidFile, nil)
+}
+
+func assertProcessTerminatedAfterMissing(t *testing.T, pidFile string, onMissing func()) {
+	t.Helper()
+
+	deadline := time.Now().Add(3 * time.Second)
+	var data []byte
+	var err error
+	missingObserved := false
+	for time.Now().Before(deadline) {
+		data, err = os.ReadFile(pidFile)
+		if err == nil {
+			break
+		}
+		if !errors.Is(err, os.ErrNotExist) {
+			t.Fatalf("read sleeper pid file: %v", err)
+		}
+		if !missingObserved {
+			missingObserved = true
+			if onMissing != nil {
+				onMissing()
+			}
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+	if err != nil {
+		t.Fatalf("read sleeper pid file before timeout: %v", err)
+	}
+	pid, err := strconv.Atoi(strings.TrimSpace(string(data)))
+	if err != nil {
+		t.Fatalf("parse sleeper pid %q: %v", data, err)
+	}
+
+	deadline = time.Now().Add(3 * time.Second)
+	for time.Now().Before(deadline) {
+		if !processAlive(pid) {
+			return
+		}
+		time.Sleep(100 * time.Millisecond)
+	}
+	t.Fatalf("sleeper process %d still alive after timeout", pid)
 }
 
 func TestLoadProviderCommandInvalidJSON(t *testing.T) {
@@ -118,6 +291,14 @@ type commandScript struct {
 	Stderr       string
 	ExitCode     int
 	SleepSeconds int
+	PidFile      string
+
+	// BackgroundSleepSeconds, if set, spawns a detached child that keeps the
+	// inherited stdout/stderr handles open well after the script itself
+	// exits, simulating a `sleep 600 & exit` style command. BackgroundPidFile
+	// records the detached child's PID.
+	BackgroundSleepSeconds int
+	BackgroundPidFile      string
 }
 
 func writeCommand(t *testing.T, script commandScript) string {
@@ -128,7 +309,11 @@ func writeCommand(t *testing.T, script commandScript) string {
 		path := filepath.Join(dir, "provider.cmd")
 		lines := []string{"@echo off"}
 		if script.SleepSeconds > 0 {
-			lines = append(lines, "powershell -NoProfile -Command \"Start-Sleep -Seconds "+itoa(script.SleepSeconds)+"\"")
+			sleep := "Start-Sleep -Seconds " + itoa(script.SleepSeconds)
+			if script.PidFile != "" {
+				sleep = "Set-Content -Path '" + psSingleQuote(script.PidFile) + "' -Value $PID -Encoding Ascii; " + sleep
+			}
+			lines = append(lines, "powershell -NoProfile -Command \""+sleep+"\"")
 		}
 		if script.Stdout != "" {
 			lines = append(lines, "echo "+script.Stdout)
@@ -136,6 +321,40 @@ func writeCommand(t *testing.T, script commandScript) string {
 		if script.Stderr != "" {
 			lines = append(lines, "echo "+script.Stderr+" 1>&2")
 		}
+		if script.BackgroundSleepSeconds > 0 {
+			readyFile := script.BackgroundPidFile + ".ready"
+			bgSleep := "Set-Content -LiteralPath '" + psSingleQuote(script.BackgroundPidFile) + "' -Value $PID -Encoding Ascii; " +
+				"Set-Content -LiteralPath '" + psSingleQuote(readyFile) + "' -Value ready -Encoding Ascii; " +
+				"Start-Sleep -Seconds " + itoa(script.BackgroundSleepSeconds)
+			lines = append(lines, "start /B powershell -NoProfile -Command \""+bgSleep+"\"")
+			// Wait for the child to record its PID before exiting, so the PID is
+			// on disk while the tree is still alive: runProviderCommand kills the
+			// tree one WaitDelay after the script exits, and a child that has not
+			// written by then never will.
+			//
+			// This waits in cmd itself. Spawning a second PowerShell just to poll
+			// cost another interpreter cold start, and that start is pure overhead
+			// added AFTER the PID is already written. On a loaded runner it pushed
+			// the script's exit past the 5s provider-command timeout, so the
+			// nonzero-exit test saw a timeout instead of the exit status it
+			// asserts. The wait is also bounded now: giving up and letting the PID
+			// assertion fail is far more diagnosable than blocking until the
+			// timeout fires and reporting an unrelated error. Each iteration is
+			// about a second, since `ping -n 2` sleeps between its two echoes,
+			// so the ceiling is roughly 15s: far longer than any cold PowerShell
+			// start seen on a runner, and far shorter than
+			// providerCommandTestBudget, so giving up surfaces a legible
+			// PID-file failure rather than a timeout that hides the real cause.
+			lines = append(lines,
+				":zeroWaitReady",
+				"if exist \""+readyFile+"\" goto zeroReady",
+				"set /a ZERO_READY_TRIES+=1",
+				"if %ZERO_READY_TRIES% GEQ 15 goto zeroReady",
+				"ping -n 2 127.0.0.1 >nul",
+				"goto zeroWaitReady",
+				":zeroReady",
+			)
+		}
 		lines = append(lines, "exit /b "+itoa(script.ExitCode))
 		if err := os.WriteFile(path, []byte(strings.Join(lines, "\r\n")), 0o700); err != nil {
 			t.Fatalf("write command: %v", err)
@@ -146,6 +365,9 @@ func writeCommand(t *testing.T, script commandScript) string {
 	path := filepath.Join(dir, "provider.sh")
 	lines := []string{"#!/bin/sh"}
 	if script.SleepSeconds > 0 {
+		if script.PidFile != "" {
+			lines = append(lines, "echo $$ > '"+shSingleQuote(script.PidFile)+"'")
+		}
 		lines = append(lines, "sleep "+itoa(script.SleepSeconds))
 	}
 	if script.Stdout != "" {
@@ -154,6 +376,10 @@ func writeCommand(t *testing.T, script commandScript) string {
 	if script.Stderr != "" {
 		lines = append(lines, "printf '%s\\n' '"+script.Stderr+"' >&2")
 	}
+	if script.BackgroundSleepSeconds > 0 {
+		lines = append(lines, "sleep "+itoa(script.BackgroundSleepSeconds)+" &")
+		lines = append(lines, "echo $! > '"+shSingleQuote(script.BackgroundPidFile)+"'")
+	}
 	lines = append(lines, "exit "+itoa(script.ExitCode))
 	if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")), 0o700); err != nil {
 		t.Fatalf("write command: %v", err)
@@ -161,6 +387,19 @@ func writeCommand(t *testing.T, script commandScript) string {
 	return path
 }
 
+// psSingleQuote escapes a value for interpolation inside a PowerShell
+// single-quoted string literal, where a literal quote is doubled.
+func psSingleQuote(value string) string {
+	return strings.ReplaceAll(value, "'", "''")
+}
+
+// shSingleQuote escapes a value for interpolation inside a POSIX shell
+// single-quoted string literal, where a literal quote must close the
+// quoted section, emit an escaped quote, then reopen it.
+func shSingleQuote(value string) string {
+	return strings.ReplaceAll(value, "'", `'\''`)
+}
+
 func itoa(value int) string {
 	if value == 0 {
 		return "0"
@@ -175,3 +414,133 @@ func itoa(value int) string {
 	}
 	return string(digits[index:])
 }
+
+// The whole point of #811: providerCommandTimeout has to be an upper bound on
+// how long the call takes, not a floor. Process creation used to run before the
+// clock started and the post-termination drain was unbounded, so a slow or
+// contended machine could block startup for far longer than the error text
+// promised.
+//
+// This one guards the end-to-end contract against gross regressions. It is NOT
+// the assertion that pins the drain bound: Terminate kills the tree, so Wait
+// returns promptly and this passes even with the drain unbounded. That property
+// is pinned by TestAwaitWithGraceGivesUp, and the start-phase bound by
+// TestRunProviderCommandCountsProcessStartAgainstTheDeadline.
+func TestRunProviderCommandBoundsTotalDuration(t *testing.T) {
+	pidFile := filepath.Join(t.TempDir(), "bound.pid")
+	command := writeCommand(t, commandScript{SleepSeconds: 60, PidFile: pidFile})
+
+	const budget = 2 * time.Second
+	start := time.Now()
+	_, _, err := runProviderCommand(command, budget)
+	elapsed := time.Since(start)
+
+	if !errors.Is(err, errProviderCommandTimeout) {
+		t.Fatalf("error = %v, want the timeout sentinel", err)
+	}
+	// Budget, plus the bounded drain, plus room for a loaded runner. A failure
+	// here means some phase is outside the deadline again, which is the bug.
+	ceiling := budget + providerCommandDrainGrace + 8*time.Second
+	if elapsed > ceiling {
+		t.Fatalf("returned after %s with a %s budget; the timeout is not bounding the call (ceiling %s)", elapsed, budget, ceiling)
+	}
+	assertProcessTerminatedIfStarted(t, pidFile)
+}
+
+// The timeout error has to carry something an operator can act on. It used to be
+// a bare string with the underlying error and the command's stderr both dropped,
+// so a user whose provider command was slow got no clue which phase ran out or
+// what the command had said.
+func TestLoadProviderCommandTimeoutReportsDiagnostics(t *testing.T) {
+	command := writeCommand(t, commandScript{
+		Stderr:       "provider is warming up",
+		SleepSeconds: 30,
+		PidFile:      filepath.Join(t.TempDir(), "diag.pid"),
+	})
+
+	_, err := LoadProviderCommand(command)
+	if err == nil {
+		t.Fatal("LoadProviderCommand() error = nil, want timeout")
+	}
+	if !strings.Contains(err.Error(), "timed out") {
+		t.Fatalf("error = %q, want a timeout message", err.Error())
+	}
+	// Wrapped, so callers can still classify it.
+	if !errors.Is(err, errProviderCommandTimeout) {
+		t.Fatalf("error = %v, want errors.Is against the timeout sentinel", err)
+	}
+	// And it must say which phase, rather than only that time ran out.
+	if !strings.Contains(err.Error(), "did not finish") && !strings.Contains(err.Error(), "starting the command") {
+		t.Fatalf("error = %q, want it to name the phase that exceeded the deadline", err.Error())
+	}
+}
+
+// The output buffers are written by cmd.Wait's I/O pump, which keeps running
+// after the timeout path has returned them to the caller, so a plain
+// bytes.Buffer would be read and written concurrently. Asserted on the buffer
+// directly: driving it end to end does not reliably produce the overlap, because
+// a terminated command usually stops writing before the snapshot is taken, so
+// such a test passes with the locking removed and proves nothing.
+//
+// Only meaningful under -race, which CI runs.
+func TestSyncBufferAllowsConcurrentWriteAndSnapshot(t *testing.T) {
+	buffer := &syncBuffer{}
+	writing := make(chan struct{})
+	go func() {
+		defer close(writing)
+		for i := 0; i < 2000; i++ {
+			_, _ = buffer.Write([]byte("x"))
+		}
+	}()
+	for i := 0; i < 2000; i++ {
+		_ = buffer.snapshot()
+	}
+	<-writing
+	if len(buffer.snapshot()) != 2000 {
+		t.Fatalf("buffer holds %d bytes, want 2000", len(buffer.snapshot()))
+	}
+}
+
+// The drain after termination must be bounded. Asserted directly on the helper,
+// because in place it is untestable without a tree that refuses to die: Wait
+// normally returns at once, which is precisely why an unbounded receive sat
+// there unnoticed. A channel that never fires is the pathological case made
+// cheap.
+func TestAwaitWithGraceGivesUp(t *testing.T) {
+	never := make(chan error)
+	start := time.Now()
+	if awaitWithGrace(never, 50*time.Millisecond) {
+		t.Fatal("reported completion for a wait that never finished")
+	}
+	if elapsed := time.Since(start); elapsed > 5*time.Second {
+		t.Fatalf("gave up after %s, want roughly the grace period", elapsed)
+	}
+	// Still reports completion when the wait does finish, or the timeout path
+	// would always claim the tree hung.
+	done := make(chan error, 1)
+	done <- nil
+	if !awaitWithGrace(done, time.Second) {
+		t.Fatal("reported expiry for a wait that had already finished")
+	}
+}
+
+// Process creation has to be inside the budget. It used to run before the clock
+// started, and on Windows that phase is CREATE_SUSPENDED plus job-object setup
+// plus a system-wide thread snapshot, none of it fast on a contended machine.
+//
+// A budget that is already spent when the call begins can only be exceeded
+// during the start, so the phase named in the error is what distinguishes the
+// two orderings: with the clock started afterwards, the start would always
+// complete first and the failure would come from the wait instead.
+func TestRunProviderCommandCountsProcessStartAgainstTheDeadline(t *testing.T) {
+	command := writeCommand(t, commandScript{Stdout: `{"name":"cmd","provider":"openai","apiKey":"sk-x","model":"m"}`})
+
+	_, _, err := runProviderCommand(command, time.Nanosecond)
+	if !errors.Is(err, errProviderCommandTimeout) {
+		t.Fatalf("error = %v, want the timeout sentinel", err)
+	}
+	if !strings.Contains(err.Error(), "starting the command") {
+		t.Fatalf("error = %q, want the start phase to be the one that exceeded the deadline; "+
+			"anything else means process creation is outside the budget again", err.Error())
+	}
+}

internal/config/export_test.go

@@ -0,0 +1,72 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package config
+
+import (
+	"encoding/json"
+	"fmt"
+	"os"
+	"strings"
+)
+
+// ToolsOverride builds a ToolsConfig that explicitly overrides the deferred-tool
+// threshold (including to 0, which disables deferral). Use this for programmatic
+// Overrides — a bare ToolsConfig{DeferThreshold: 0} is indistinguishable from
+// "unset" and will not override.
+func ToolsOverride(deferThreshold int) ToolsConfig {
+	return ToolsConfig{DeferThreshold: deferThreshold, deferThresholdSet: true}
+}
+
+// ValidateFile reads and parses path as a Zero FileConfig and runs the same
+// semantic provider/model rules used during resolution. It returns the parsed
+// config (zero value on parse failure) plus any structured issues. A parse
+// failure yields a single issue whose Message is the underlying JSON error's
+// text (already flattened via Error(), not chained — callers cannot recover
+// the original *json.SyntaxError / *json.UnmarshalTypeError via errors.As).
+func ValidateFile(path string) (FileConfig, []Issue) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return FileConfig{}, []Issue{{Message: fmt.Sprintf("read config %s: %v", path, err)}}
+	}
+
+	var cfg FileConfig
+	if err := json.Unmarshal(data, &cfg); err != nil {
+		return FileConfig{}, []Issue{{Message: fmt.Sprintf("invalid config JSON %s: %v", path, err)}}
+	}
+
+	issues := validateSemantics(cfg)
+	issues = append(issues, unknownFieldIssues(data)...)
+	return cfg, issues
+}
+
+// SetProviderDescription sets a provider's description VERBATIM — including to
+// empty. The generic UpsertProvider merge treats empty fields as "leave
+// unchanged", so clearing a description needs this dedicated setter.
+func SetProviderDescription(path string, name string, description string) (FileConfig, error) {
+	path = strings.TrimSpace(path)
+	if path == "" {
+		return FileConfig{}, fmt.Errorf("config path is required")
+	}
+	name = strings.TrimSpace(name)
+	if name == "" {
+		return FileConfig{}, fmt.Errorf("provider name is required")
+	}
+
+	data, err := os.ReadFile(path)
+	if err != nil {
+		return FileConfig{}, fmt.Errorf("read config %s: %w", path, err)
+	}
+	cfg := FileConfig{}
+	if err := json.Unmarshal(data, &cfg); err != nil {
+		return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err)
+	}
+	for index := range cfg.Providers {
+		if strings.EqualFold(strings.TrimSpace(cfg.Providers[index].Name), name) {
+			cfg.Providers[index].Description = strings.TrimSpace(description)
+			if err := writeConfigFile(path, cfg); err != nil {
+				return FileConfig{}, err
+			}
+			return cfg, nil
+		}
+	}
+	return FileConfig{}, fmt.Errorf("provider %q not found", name)
+}

internal/config/mcp_merge.go

@@ -87,7 +87,7 @@ func mergeMCPServer(base MCPServerConfig, next MCPServerConfig, canReenable bool
 		// server the user explicitly enabled. The only layer allowed to
 		// override an explicit higher-scope decision is the CLI override scope
 		// (canReenable=true).
-		if canReenable || !(baseDisabledSet && baseDisabled != next.Disabled) {
+		if canReenable || !baseDisabledSet || baseDisabled == next.Disabled {
 			base.Disabled = next.Disabled
 		}
 	}

internal/config/process_posix.go

@@ -3,18 +3,59 @@
 package config
 
 import (
+	"io"
 	"os/exec"
 	"syscall"
 )
 
-func configureCommandProcess(cmd *exec.Cmd) {
-	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+type commandProcess struct {
+	groupID     int
+	anchor      *exec.Cmd
+	anchorInput io.WriteCloser
 }
 
-func terminateCommandProcess(cmd *exec.Cmd) {
-	if cmd.Process == nil {
-		return
+// startCommandProcess retains a live process-group leader until cleanup. The
+// provider shell may be reaped before Wait finishes draining descendant-held
+// pipes, so its PID cannot safely identify the group after Wait returns.
+func startCommandProcess(cmd *exec.Cmd) (*commandProcess, error) {
+	anchor := exec.Command("sh", "-c", "read _")
+	anchor.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+	anchorInput, err := anchor.StdinPipe()
+	if err != nil {
+		return nil, err
 	}
-	_ = syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)
-	_ = cmd.Process.Kill()
+	if err := anchor.Start(); err != nil {
+		_ = anchorInput.Close()
+		return nil, err
+	}
+
+	proc := &commandProcess{groupID: anchor.Process.Pid, anchor: anchor, anchorInput: anchorInput}
+	if cmd.SysProcAttr == nil {
+		cmd.SysProcAttr = &syscall.SysProcAttr{}
+	}
+	cmd.SysProcAttr.Setpgid = true
+	cmd.SysProcAttr.Pgid = proc.groupID
+	if err := cmd.Start(); err != nil {
+		proc.Close()
+		return nil, err
+	}
+	return proc, nil
+}
+
+func (p *commandProcess) Terminate() {
+	if p.groupID != 0 {
+		_ = syscall.Kill(-p.groupID, syscall.SIGKILL)
+	}
+}
+
+func (p *commandProcess) Close() {
+	if p.anchorInput != nil {
+		_ = p.anchorInput.Close()
+		p.anchorInput = nil
+	}
+	if p.anchor != nil {
+		_ = p.anchor.Wait()
+		p.anchor = nil
+	}
+	p.groupID = 0
 }

internal/config/process_posix_test.go

@@ -0,0 +1,66 @@
+//go:build !windows
+
+package config
+
+import (
+	"os/exec"
+	"syscall"
+	"testing"
+)
+
+func TestCommandProcessPreservesSysProcAttr(t *testing.T) {
+	cmd := exec.Command("sh", "-c", "exit 0")
+	attr := &syscall.SysProcAttr{}
+	cmd.SysProcAttr = attr
+
+	proc, err := startCommandProcess(cmd)
+	if err != nil {
+		t.Fatalf("startCommandProcess: %v", err)
+	}
+	defer proc.Close()
+
+	if cmd.SysProcAttr != attr {
+		t.Fatal("startCommandProcess replaced SysProcAttr")
+	}
+	if !cmd.SysProcAttr.Setpgid || cmd.SysProcAttr.Pgid != proc.groupID {
+		t.Fatalf("process group = Setpgid %v, Pgid %d; want true, %d", cmd.SysProcAttr.Setpgid, cmd.SysProcAttr.Pgid, proc.groupID)
+	}
+	if err := cmd.Wait(); err != nil {
+		t.Fatalf("Wait: %v", err)
+	}
+}
+
+func TestCommandProcessRetainsGroupIdentityAfterProviderWait(t *testing.T) {
+	cmd := exec.Command("sh", "-c", "exit 7")
+	proc, err := startCommandProcess(cmd)
+	if err != nil {
+		t.Fatalf("startCommandProcess: %v", err)
+	}
+	defer func() {
+		proc.Terminate()
+		proc.Close()
+	}()
+
+	providerPID := cmd.Process.Pid
+	anchorPID := proc.groupID
+	if err := cmd.Wait(); err == nil {
+		t.Fatal("Wait error = nil, want nonzero exit")
+	}
+	if anchorPID == providerPID {
+		t.Fatalf("group identity reused provider PID %d", providerPID)
+	}
+	if !processAlive(anchorPID) {
+		t.Fatalf("group leader %d exited before cleanup", anchorPID)
+	}
+
+	proc.Terminate()
+	proc.Close()
+	if processAlive(anchorPID) {
+		t.Fatalf("group leader %d still alive after cleanup", anchorPID)
+	}
+}
+
+func processAlive(pid int) bool {
+	err := syscall.Kill(pid, 0)
+	return err == nil || err == syscall.EPERM
+}

internal/config/process_windows.go

@@ -3,23 +3,165 @@
 package config
 
 import (
+	"context"
 	"os"
 	"os/exec"
 	"path/filepath"
 	"strconv"
+	"syscall"
+	"time"
+	"unsafe"
+
+	"golang.org/x/sys/windows"
 )
 
-func configureCommandProcess(cmd *exec.Cmd) {}
+// configureCommandProcess starts the process suspended so it can be
+// assigned to the job object before its main thread (and therefore any
+// code it runs) executes. Without this, a fast command can spawn and
+// detach a descendant before AssignProcessToJobObject runs, letting that
+// descendant escape the job and survive termination.
+func configureCommandProcess(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{CreationFlags: windows.CREATE_SUSPENDED}
+}
 
-func terminateCommandProcess(cmd *exec.Cmd) {
+func startCommandProcess(cmd *exec.Cmd) (*commandProcess, error) {
+	configureCommandProcess(cmd)
+	if err := cmd.Start(); err != nil {
+		return nil, err
+	}
+	return attachCommandProcess(cmd), nil
+}
+
+// commandProcess tracks a started provider command so its entire process
+// tree can be terminated on timeout. It prefers a job object: taskkill /T
+// walks the tree by parent PID in user space and can miss descendants,
+// letting them run to completion while Wait blocks on inherited pipes.
+type commandProcess struct {
+	cmd           *exec.Cmd
+	job           windows.Handle
+	processHandle windows.Handle
+}
+
+func attachCommandProcess(cmd *exec.Cmd) *commandProcess {
+	proc := &commandProcess{cmd: cmd}
 	if cmd.Process == nil {
+		return proc
+	}
+	// The main thread is suspended (see configureCommandProcess); resume it
+	// once we're done attaching, however that turns out. If resuming fails
+	// outright (Toolhelp/OpenThread/ResumeThread errors are otherwise
+	// swallowed), the process would sit suspended forever and never exit on
+	// its own, making the failure look like an ordinary 5s provider-command
+	// timeout. Terminate it immediately instead so the real cause surfaces
+	// quickly and no suspended process is left behind.
+	defer func() {
+		if !resumeMainThread(cmd.Process.Pid) {
+			proc.Terminate()
+		}
+	}()
+
+	handle, err := windows.OpenProcess(windows.PROCESS_SET_QUOTA|windows.PROCESS_TERMINATE|windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(cmd.Process.Pid))
+	if err != nil {
+		return proc
+	}
+	proc.processHandle = handle
+
+	job, err := windows.CreateJobObject(nil, nil)
+	if err != nil {
+		return proc
+	}
+	if err := windows.AssignProcessToJobObject(job, handle); err != nil {
+		_ = windows.CloseHandle(job)
+		return proc
+	}
+	proc.job = job
+	return proc
+}
+
+// resumeMainThread resumes every (assumed suspended) thread of pid and
+// reports whether at least one was actually resumed. Callers must treat a
+// false result as a failed resume, not a benign no-op: a suspended process
+// that's never woken up will neither exit nor produce output on its own.
+func resumeMainThread(pid int) bool {
+	snapshot, err := windows.CreateToolhelp32Snapshot(windows.TH32CS_SNAPTHREAD, 0)
+	if err != nil {
+		return false
+	}
+	defer func() { _ = windows.CloseHandle(snapshot) }()
+
+	var entry windows.ThreadEntry32
+	entry.Size = uint32(unsafe.Sizeof(entry))
+	resumed := false
+	for err := windows.Thread32First(snapshot, &entry); err == nil; err = windows.Thread32Next(snapshot, &entry) {
+		if entry.OwnerProcessID != uint32(pid) {
+			continue
+		}
+		thread, err := windows.OpenThread(windows.THREAD_SUSPEND_RESUME, false, entry.ThreadID)
+		if err != nil {
+			continue
+		}
+		if _, err := windows.ResumeThread(thread); err == nil {
+			resumed = true
+		}
+		_ = windows.CloseHandle(thread)
+	}
+	return resumed
+}
+
+func (p *commandProcess) Terminate() {
+	if p.job != 0 {
+		// A job handle means the process tree was contained from launch, so
+		// TerminateJobObject alone kills every member atomically. Don't also
+		// fall through to the taskkill/PID path below: by the time
+		// Terminate runs on the ErrWaitDelay path, cmd.Wait has already
+		// reaped the root process, so its PID may have been reused by an
+		// unrelated process, and forcing /T /PID against a reused PID would
+		// kill the wrong tree.
+		_ = windows.TerminateJobObject(p.job, 1)
 		return
 	}
-	taskkill := taskkillPath()
-	_ = exec.Command(taskkill, "/T", "/F", "/PID", strconv.Itoa(cmd.Process.Pid)).Run()
-	_ = cmd.Process.Kill()
+	if p.cmd.Process == nil {
+		return
+	}
+	if p.processHandle == 0 {
+		// Without a retained identity, never target a numeric PID that may
+		// have been reaped and reused. os.Process.Kill uses Go's original
+		// process state instead.
+		_ = p.cmd.Process.Kill()
+		return
+	}
+	// Fallback for the rare case where job creation or assignment failed:
+	// only invoke taskkill while the identity-preserving handle confirms the
+	// original root is still active. Keeping that handle open through Run
+	// prevents Windows from reusing its PID for an unrelated process.
+	var exitCode uint32
+	if err := windows.GetExitCodeProcess(p.processHandle, &exitCode); err == nil && exitCode == stillActive {
+		taskkill := taskkillPath()
+		ctx, cancel := context.WithTimeout(context.Background(), 2*time.Second)
+		_ = exec.CommandContext(ctx, taskkill, "/T", "/F", "/PID", strconv.Itoa(p.cmd.Process.Pid)).Run()
+		cancel()
+	}
+	_ = windows.TerminateProcess(p.processHandle, 1)
 }
 
+// Close releases the retained handles without touching any still-running
+// descendants: the job carries no KILL_ON_JOB_CLOSE limit, so on the success
+// path a provider command's detached helpers keep running exactly as they did
+// before job objects were introduced. Descendant termination happens
+// explicitly via Terminate, called only on timeout/error.
+func (p *commandProcess) Close() {
+	if p.job != 0 {
+		_ = windows.CloseHandle(p.job)
+		p.job = 0
+	}
+	if p.processHandle != 0 {
+		_ = windows.CloseHandle(p.processHandle)
+		p.processHandle = 0
+	}
+}
+
+const stillActive = uint32(259) // STILL_ACTIVE
+
 func taskkillPath() string {
 	systemRoot := os.Getenv("SystemRoot")
 	if systemRoot == "" {

internal/config/process_windows_test.go

@@ -0,0 +1,65 @@
+//go:build windows
+
+package config
+
+import (
+	"os/exec"
+	"testing"
+
+	"golang.org/x/sys/windows"
+)
+
+// TestResumeMainThreadReportsFailureForUnknownPID covers the contract
+// attachCommandProcess relies on: when no thread belonging to pid can be
+// found (here, because the pid doesn't correspond to a live process),
+// resumeMainThread must report false rather than silently doing nothing,
+// so a caller can react (e.g. terminate the process) instead of leaving it
+// suspended indefinitely.
+func TestResumeMainThreadReportsFailureForUnknownPID(t *testing.T) {
+	if resumeMainThread(0) {
+		t.Fatal("resumeMainThread(0) = true, want false for a pid with no threads")
+	}
+}
+
+func TestCommandProcessRetainsIdentityAfterWait(t *testing.T) {
+	cmd := exec.Command("cmd", "/C", "exit /b 0")
+	configureCommandProcess(cmd)
+	if err := cmd.Start(); err != nil {
+		t.Fatalf("Start: %v", err)
+	}
+	proc := attachCommandProcess(cmd)
+	t.Cleanup(proc.Close)
+	if proc.processHandle == 0 {
+		t.Fatal("attachCommandProcess did not retain a process handle")
+	}
+	if err := cmd.Wait(); err != nil {
+		t.Fatalf("Wait: %v", err)
+	}
+
+	var code uint32
+	if err := windows.GetExitCodeProcess(proc.processHandle, &code); err != nil {
+		t.Fatalf("GetExitCodeProcess after Wait: %v", err)
+	}
+	if code == stillActive {
+		t.Fatalf("exit code = STILL_ACTIVE after Wait")
+	}
+
+	proc.Close()
+	if proc.job != 0 || proc.processHandle != 0 {
+		t.Fatalf("Close left handles open: job=%v process=%v", proc.job, proc.processHandle)
+	}
+}
+
+func processAlive(pid int) bool {
+	handle, err := windows.OpenProcess(windows.PROCESS_QUERY_LIMITED_INFORMATION, false, uint32(pid))
+	if err != nil {
+		return false
+	}
+	defer func() { _ = windows.CloseHandle(handle) }()
+
+	var code uint32
+	if err := windows.GetExitCodeProcess(handle, &code); err != nil {
+		return false
+	}
+	return code == stillActive
+}

internal/config/types.go

@@ -113,7 +113,7 @@ type PreferencesConfig struct {
 	// ZERO_THEME, so a /theme choice survives restart. Empty = unset (defaults auto).
 	Theme string `json:"theme,omitempty"`
 	// Recaps is a tri-state: nil (unset) defaults to ON; an explicit false means
-	// the user turned post-turn recaps off. A *bool is its own tri-state, so no
+	// the user turned idle recaps off. A *bool is its own tri-state, so no
 	// custom unmarshal is needed (unlike ToolsConfig.DeferThreshold's int).
 	Recaps *bool `json:"recaps,omitempty"`
 }
@@ -130,7 +130,7 @@ type RecentModelEntry struct {
 // MaxRecentModels caps the persisted/displayed recent-selection history.
 const MaxRecentModels = 5
 
-// RecapsEnabled reports whether post-turn recaps are on. Unset defaults to ON.
+// RecapsEnabled reports whether idle recaps are on. Unset defaults to ON.
 func (p PreferencesConfig) RecapsEnabled() bool {
 	return p.Recaps == nil || *p.Recaps
 }
@@ -322,14 +322,6 @@ type SwarmConfig struct {
 	MaxTeamSize int `json:"maxTeamSize,omitempty"`
 }
 
-// ToolsOverride builds a ToolsConfig that explicitly overrides the deferred-tool
-// threshold (including to 0, which disables deferral). Use this for programmatic
-// Overrides — a bare ToolsConfig{DeferThreshold: 0} is indistinguishable from
-// "unset" and will not override.
-func ToolsOverride(deferThreshold int) ToolsConfig {
-	return ToolsConfig{DeferThreshold: deferThreshold, deferThresholdSet: true}
-}
-
 func (cfg *ToolsConfig) UnmarshalJSON(data []byte) error {
 	type rawTools struct {
 		DeferThreshold *int `json:"deferThreshold"`

internal/config/validate.go

@@ -3,7 +3,6 @@ package config
 import (
 	"encoding/json"
 	"fmt"
-	"os"
 )
 
 // Issue is a single structured problem found while validating a config file.
@@ -13,28 +12,6 @@ type Issue struct {
 	Message   string `json:"message"`
 }
 
-// ValidateFile reads and parses path as a Zero FileConfig and runs the same
-// semantic provider/model rules used during resolution. It returns the parsed
-// config (zero value on parse failure) plus any structured issues. A parse
-// failure yields a single issue whose Message wraps the underlying JSON error
-// so callers can extract *json.SyntaxError / *json.UnmarshalTypeError offsets
-// via errors.As.
-func ValidateFile(path string) (FileConfig, []Issue) {
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return FileConfig{}, []Issue{{Message: fmt.Sprintf("read config %s: %v", path, err)}}
-	}
-
-	var cfg FileConfig
-	if err := json.Unmarshal(data, &cfg); err != nil {
-		return FileConfig{}, []Issue{{Message: fmt.Errorf("invalid config JSON %s: %w", path, err).Error()}}
-	}
-
-	issues := validateSemantics(cfg)
-	issues = append(issues, unknownFieldIssues(data)...)
-	return cfg, issues
-}
-
 // ValidateBytes parses data as a Zero FileConfig and runs the same semantic
 // provider/model rules as ValidateFile. It returns the parsed config (zero
 // value on parse failure) plus any structured issues. A parse failure yields a

internal/config/writer.go

@@ -431,39 +431,6 @@ func EditProvider(path string, edit ProviderEdit) (FileConfig, error) {
 	return cfg, nil
 }
 
-// SetProviderDescription sets a provider's description VERBATIM — including to
-// empty. The generic UpsertProvider merge treats empty fields as "leave
-// unchanged", so clearing a description needs this dedicated setter.
-func SetProviderDescription(path string, name string, description string) (FileConfig, error) {
-	path = strings.TrimSpace(path)
-	if path == "" {
-		return FileConfig{}, fmt.Errorf("config path is required")
-	}
-	name = strings.TrimSpace(name)
-	if name == "" {
-		return FileConfig{}, fmt.Errorf("provider name is required")
-	}
-
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return FileConfig{}, fmt.Errorf("read config %s: %w", path, err)
-	}
-	cfg := FileConfig{}
-	if err := json.Unmarshal(data, &cfg); err != nil {
-		return FileConfig{}, fmt.Errorf("invalid config JSON %s: %w", path, err)
-	}
-	for index := range cfg.Providers {
-		if strings.EqualFold(strings.TrimSpace(cfg.Providers[index].Name), name) {
-			cfg.Providers[index].Description = strings.TrimSpace(description)
-			if err := writeConfigFile(path, cfg); err != nil {
-				return FileConfig{}, err
-			}
-			return cfg, nil
-		}
-	}
-	return FileConfig{}, fmt.Errorf("provider %q not found", name)
-}
-
 // migrateStoredProviderKey moves a credential-store entry to a new provider
 // name: write-new-then-delete-old, so an interruption can leave a duplicate but
 // never a missing key. A missing source entry is a no-op (the marker may be
@@ -579,7 +546,7 @@ func SetRecentModels(path string, entries []RecentModelEntry) (FileConfig, error
 	return cfg, nil
 }
 
-// SetRecapsEnabled persists the post-turn recap preference, mirroring
+// SetRecapsEnabled persists the idle recap preference, mirroring
 // SetFavoriteModels (read-modify-atomic-write).
 func SetRecapsEnabled(path string, enabled bool) (FileConfig, error) {
 	path = strings.TrimSpace(path)

internal/contextreport/contextreport.go

@@ -40,6 +40,7 @@ type Options struct {
 	Provider            config.ProviderProfile
 	Registry            *tools.Registry
 	ContextWindow       int
+	DeferThreshold      int
 	ProjectContextFiles []string
 }
 
@@ -56,6 +57,8 @@ type Report struct {
 	FreeTokens           int        `json:"freeTokens"`
 	UsedFraction         float64    `json:"usedFraction"`
 	ToolCount            int        `json:"toolCount"`
+	AvailableToolCount   int        `json:"availableToolCount,omitempty"`
+	DeferredToolCount    int        `json:"deferredToolCount,omitempty"`
 	ProjectGuidelineFile string     `json:"projectGuidelineFile,omitempty"`
 	Categories           []Category `json:"categories"`
 }
@@ -113,8 +116,10 @@ func Build(options Options) (Report, error) {
 		categories = append(categories, category(CategoryWorkspaceMap, "Workspace map", estimateTextTokens(workspaceMap), report.ContextWindow))
 	}
 
-	toolCount, toolTokens := estimateRegistryTools(options.Registry)
+	toolCount, availableTools, deferredTools, toolTokens := estimateRegistryTools(options.Registry, options.DeferThreshold)
 	report.ToolCount = toolCount
+	report.AvailableToolCount = availableTools
+	report.DeferredToolCount = deferredTools
 	if toolTokens > 0 {
 		categories = append(categories, category(CategoryTools, "Tools", toolTokens, report.ContextWindow))
 	}
@@ -175,7 +180,11 @@ func Format(report Report) string {
 		lines = append(lines, fmt.Sprintf("usage: %s tokens (context window unknown)", compactNumber(report.UsedTokens)))
 	}
 	if report.ToolCount > 0 {
-		lines = append(lines, fmt.Sprintf("tools: %d", report.ToolCount))
+		toolLine := fmt.Sprintf("tools: %d exposed", report.ToolCount)
+		if report.AvailableToolCount > report.ToolCount {
+			toolLine += fmt.Sprintf(", %d available, %d deferred", report.AvailableToolCount, report.DeferredToolCount)
+		}
+		lines = append(lines, toolLine)
 	}
 	if report.ProjectGuidelineFile != "" {
 		lines = append(lines, "project_guidelines: "+report.ProjectGuidelineFile)
@@ -212,29 +221,51 @@ func category(key string, name string, tokens int, contextWindow int) Category {
 	return cat
 }
 
-func estimateRegistryTools(registry *tools.Registry) (int, int) {
+func estimateRegistryTools(registry *tools.Registry, deferThreshold int) (int, int, int, int) {
 	if registry == nil {
-		return 0, 0
+		return 0, 0, 0, 0
 	}
 	all := registry.All()
 	sort.Slice(all, func(left int, right int) bool {
 		return all[left].Name() < all[right].Name()
 	})
-	total := 0
-	count := 0
+	var visible []tools.Tool
+	var deferred []tools.Tool
 	for _, tool := range all {
 		if tool.Safety().Permission == tools.PermissionDeny {
 			continue
 		}
-		count++
+		visible = append(visible, tool)
+		if tools.IsDeferred(tool) {
+			deferred = append(deferred, tool)
+		}
+	}
+	active := deferThreshold > 0 && len(deferred) >= deferThreshold
+	exposed := visible
+	if active {
+		exposed = make([]tools.Tool, 0, len(visible)-len(deferred)+1)
+		for _, tool := range visible {
+			if !tools.IsDeferred(tool) && tool.Name() != tools.ToolSearchToolName {
+				exposed = append(exposed, tool)
+			}
+		}
+		exposed = append(exposed, tools.NewToolSearchTool(registry))
+	}
+
+	total := 0
+	for _, tool := range exposed {
 		total += estimateTextTokens(tool.Name())
-		total += estimateTextTokens(tool.Description())
+		description := tool.Description()
+		if active && tool.Name() == tools.ToolSearchToolName {
+			description = tools.BuildToolSearchDescription(deferred)
+		}
+		total += estimateTextTokens(description)
 		if encoded, err := json.Marshal(tool.Parameters()); err == nil {
 			total += estimateTextTokens(string(encoded))
 		}
 		total += toolDefinitionOverheadTokens
 	}
-	return count, total
+	return len(exposed), len(visible), len(deferred), total
 }
 
 func readProjectGuidelines(root string, names []string) (string, string) {

internal/contextreport/contextreport_test.go

@@ -16,7 +16,7 @@ func TestBuildCountsProjectGuidelinesAndFreeBudget(t *testing.T) {
 	writeTestFile(t, root, "AGENTS.md", strings.Repeat("project rules\n", 200))
 
 	registry := tools.NewRegistry()
-	for _, tool := range tools.CoreTools(root) {
+	for _, tool := range tools.CoreToolsScoped(root, nil) {
 		registry.Register(tool)
 	}
 
@@ -80,7 +80,7 @@ func TestBuildHasStableJSONContractAndCategoryMath(t *testing.T) {
 	writeTestFile(t, root, "ZERO.md", strings.Repeat("zero rules\n", 16))
 
 	registry := tools.NewRegistry()
-	for _, tool := range tools.CoreTools(root) {
+	for _, tool := range tools.CoreToolsScoped(root, nil) {
 		registry.Register(tool)
 	}
 
@@ -130,6 +130,35 @@ func TestBuildHasStableJSONContractAndCategoryMath(t *testing.T) {
 	}
 }
 
+func TestBuildReportsInitialDeferredToolSurface(t *testing.T) {
+	root := t.TempDir()
+	registry := tools.NewRegistry()
+	for _, tool := range tools.CoreToolsScoped(root, nil) {
+		registry.Register(tool)
+	}
+
+	eager, err := Build(Options{WorkspaceRoot: root, Registry: registry})
+	if err != nil {
+		t.Fatal(err)
+	}
+	deferred, err := Build(Options{WorkspaceRoot: root, Registry: registry, DeferThreshold: 3})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if deferred.DeferredToolCount < 3 {
+		t.Fatalf("DeferredToolCount = %d, want at least 3", deferred.DeferredToolCount)
+	}
+	if deferred.AvailableToolCount != eager.ToolCount {
+		t.Fatalf("available tools = %d, want eager total %d", deferred.AvailableToolCount, eager.ToolCount)
+	}
+	if deferred.ToolCount >= deferred.AvailableToolCount {
+		t.Fatalf("initial exposed tools = %d, available = %d; expected a smaller initial surface", deferred.ToolCount, deferred.AvailableToolCount)
+	}
+	if categoryByKey(deferred, CategoryTools).Tokens >= categoryByKey(eager, CategoryTools).Tokens {
+		t.Fatalf("deferred tool schemas did not reduce the initial context")
+	}
+}
+
 func TestBuildClampsFreeBudgetWhenOverContextWindow(t *testing.T) {
 	root := t.TempDir()
 	writeTestFile(t, root, "AGENTS.md", strings.Repeat("very large project rule\n", 1000))

internal/cron/export_test.go

@@ -0,0 +1,4 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package cron
+
+func (s Schedule) String() string { return s.expr }

internal/cron/schedule.go

@@ -23,8 +23,6 @@ type Schedule struct {
 	expr    string
 }
 
-func (s Schedule) String() string { return s.expr }
-
 // full returns a bitset with every value in [min,max] set.
 func full(min, max int) uint64 {
 	var m uint64

internal/cron/store.go

@@ -99,35 +99,60 @@ func (s *Store) Add(job Job) (Job, error) {
 		job.Status = StatusActive
 	}
 	job.CreatedAt = s.now().UTC()
-	id, err := s.allocID()
+	id, unlock, err := s.reserveID()
 	if err != nil {
 		return Job{}, err
 	}
+	// Held from reservation through initial persistence: without this, Remove
+	// (which takes the same per-ID lock before deleting) could delete the
+	// bare reservation directory the instant after Mkdir and report success,
+	// then writeJob's MkdirAll would silently resurrect it and persist the
+	// job anyway — or a second Add could re-reserve the freed id while this
+	// writer is still running, putting two writers on the same metadata path.
+	defer unlock()
 	job.ID = id
-	if err := s.writeJob(job); err != nil {
+	if err := s.writeReservedJob(job); err != nil {
 		return Job{}, err
 	}
 	return job, nil
 }
 
-func (s *Store) allocID() (string, error) {
+// reserveID atomically reserves an unused job id (by os.Mkdir-ing its
+// directory) and returns the per-ID lock held for that id, still acquired.
+// The caller must release it (via the returned unlock) only once the
+// reservation has been committed (metadata.json written) or torn down —
+// see Add.
+func (s *Store) reserveID() (string, func(), error) {
+	if err := os.MkdirAll(s.root, 0o700); err != nil {
+		return "", nil, fmt.Errorf("create cron store root: %w", err)
+	}
 	base := s.now().UTC().Format("20060102-150405")
 	for n := 0; n < 100; n++ {
 		id := base
 		if n > 0 {
 			id = fmt.Sprintf("%s-%d", base, n)
 		}
-		if _, err := os.Stat(filepath.Join(s.root, id)); errors.Is(err, os.ErrNotExist) {
-			return id, nil
+		unlock, err := s.lockJob(id)
+		if err != nil {
+			return "", nil, err
 		}
+		mkErr := os.Mkdir(s.jobDir(id), 0o700)
+		if mkErr == nil {
+			return id, unlock, nil
+		}
+		unlock()
+		if errors.Is(mkErr, os.ErrExist) {
+			continue
+		}
+		return "", nil, fmt.Errorf("reserve cron job id %q: %w", id, mkErr)
 	}
-	return "", errors.New("could not allocate a unique cron job id")
+	return "", nil, errors.New("could not allocate a unique cron job id")
 }
 
 func (s *Store) jobDir(id string) string { return filepath.Join(s.root, id) }
 
 // validID rejects ids that could escape the store root (path separators or
-// traversal). allocID-generated timestamp ids always pass; this guards
+// traversal). reserveID-generated timestamp ids always pass; this guards
 // externally-supplied ids (get/update/remove/append).
 func validID(id string) bool {
 	return id != "" && id != "." && id != ".." && !strings.ContainsAny(id, `/\`) && filepath.Base(id) == id
@@ -153,6 +178,16 @@ func (s *Store) writeJob(job Job) error {
 	return nil
 }
 
+func (s *Store) writeReservedJob(job Job) error {
+	if err := s.writeJob(job); err != nil {
+		if cleanupErr := os.RemoveAll(s.jobDir(job.ID)); cleanupErr != nil {
+			return errors.Join(err, fmt.Errorf("remove failed cron job reservation: %w", cleanupErr))
+		}
+		return err
+	}
+	return nil
+}
+
 func (s *Store) Get(id string) (Job, error) {
 	if !validID(id) {
 		return Job{}, fmt.Errorf("invalid cron job id %q", id)

internal/cron/store_test.go

@@ -2,9 +2,11 @@ package cron
 
 import (
 	"errors"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"testing"
 	"time"
 )
@@ -74,6 +76,160 @@ func TestStoreAddListGetRemove(t *testing.T) {
 	}
 }
 
+func TestStoreAddReservesUniqueIDsAcrossConcurrentStores(t *testing.T) {
+	root := t.TempDir()
+	now := func() time.Time { return time.Unix(1000, 0).UTC() }
+	const adds = 32
+
+	start := make(chan struct{})
+	results := make(chan Job, adds)
+	errs := make(chan error, adds)
+	var wg sync.WaitGroup
+	for i := 0; i < adds; i++ {
+		wg.Add(1)
+		go func(index int) {
+			defer wg.Done()
+			store := NewStore(StoreOptions{RootDir: root, Now: now})
+			<-start
+			job, err := store.Add(Job{Expr: "* * * * *", Prompt: fmt.Sprintf("job-%d", index)})
+			if err != nil {
+				errs <- err
+				return
+			}
+			results <- job
+		}(i)
+	}
+	close(start)
+	wg.Wait()
+	close(results)
+	close(errs)
+
+	for err := range errs {
+		t.Errorf("concurrent Add: %v", err)
+	}
+	seen := make(map[string]struct{}, adds)
+	reader := NewStore(StoreOptions{RootDir: root, Now: now})
+	for job := range results {
+		if _, duplicate := seen[job.ID]; duplicate {
+			t.Errorf("concurrent Add returned duplicate ID %q", job.ID)
+		}
+		seen[job.ID] = struct{}{}
+		persisted, err := reader.Get(job.ID)
+		if err != nil {
+			t.Errorf("Get(%q): %v", job.ID, err)
+			continue
+		}
+		if persisted.Prompt != job.Prompt {
+			t.Errorf("Get(%q).Prompt = %q, want %q", job.ID, persisted.Prompt, job.Prompt)
+		}
+	}
+	if len(seen) != adds {
+		t.Fatalf("unique jobs = %d, want %d", len(seen), adds)
+	}
+}
+
+// TestAddDoesNotLeakReservationToConcurrentRemove reproduces the audit
+// finding that Remove could observe and delete an in-flight Add's bare
+// reservation directory (Mkdir'd but not yet holding metadata.json) and
+// report success — after which Add's writeJob would silently resurrect the
+// directory and persist the job anyway. Add now holds the same per-ID lock
+// Remove takes, from reservation through initial persistence, so a
+// concurrent Remove for that id must wait until the reservation is either
+// committed or torn down, never observing the bare reservation in between.
+func TestAddDoesNotLeakReservationToConcurrentRemove(t *testing.T) {
+	store := newTestStore(t)
+
+	// Simulate the in-flight window Add now holds the lock across: the id is
+	// reserved (its directory exists) but metadata.json has not been written.
+	id, unlock, err := store.reserveID()
+	if err != nil {
+		t.Fatalf("reserveID: %v", err)
+	}
+	if _, statErr := os.Stat(store.jobDir(id)); statErr != nil {
+		t.Fatalf("reservation directory missing: %v", statErr)
+	}
+
+	removeErr := make(chan error, 1)
+	go func() {
+		removeErr <- store.Remove(id)
+	}()
+
+	// Remove cannot proceed past the shared per-ID lock we're holding, so it's
+	// safe to commit the job here, before releasing it: if Remove could race
+	// ahead of this write, it would delete the bare reservation instead of
+	// waiting to observe (and remove) the committed job.
+	job := Job{ID: id, Expr: "* * * * *", Prompt: "job", Status: StatusActive}
+	if err := store.writeJob(job); err != nil {
+		t.Fatalf("writeJob: %v", err)
+	}
+	unlock()
+
+	if err := <-removeErr; err != nil {
+		t.Fatalf("Remove of the committed job: %v", err)
+	}
+	if _, err := os.Stat(store.jobDir(id)); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("job directory should be gone after Remove: %v", err)
+	}
+	if _, err := store.Get(id); !errors.Is(err, ErrJobNotFound) {
+		t.Fatalf("Get after Remove = %v, want ErrJobNotFound", err)
+	}
+}
+
+// TestReserveIDSerializesOnPerIDLock covers the other half of the audit
+// finding: a second Add attempting the SAME candidate id while the first
+// reservation is still held must block on the shared per-ID lock (not just
+// race the directory Mkdir), so it never ends up writing metadata.json for
+// an id whose first writer is still in flight. It moves on to the next
+// candidate id instead.
+func TestReserveIDSerializesOnPerIDLock(t *testing.T) {
+	store := newTestStore(t)
+
+	id, unlock, err := store.reserveID()
+	if err != nil {
+		t.Fatalf("first reserveID: %v", err)
+	}
+
+	type reservation struct {
+		id  string
+		err error
+	}
+	second := make(chan reservation, 1)
+	go func() {
+		id2, unlock2, err := store.reserveID()
+		if err == nil {
+			unlock2()
+		}
+		second <- reservation{id: id2, err: err}
+	}()
+
+	// The second reserveID cannot observe an id until this lock is released:
+	// it contends on the same per-ID lock file before ever attempting Mkdir.
+	unlock()
+
+	got := <-second
+	if got.err != nil {
+		t.Fatalf("second reserveID: %v", got.err)
+	}
+	if got.id == id {
+		t.Fatalf("second reserveID returned the first id %q while its reservation was still held", got.id)
+	}
+}
+
+func TestWriteReservedJobRemovesReservationOnFailure(t *testing.T) {
+	store := newTestStore(t)
+	job := Job{ID: "reserved", Expr: "* * * * *", Prompt: "job"}
+	if err := os.MkdirAll(filepath.Join(store.jobDir(job.ID), "metadata.json.tmp"), 0o700); err != nil {
+		t.Fatal(err)
+	}
+
+	if err := store.writeReservedJob(job); err == nil {
+		t.Fatal("writeReservedJob error = nil, want metadata write failure")
+	}
+	if _, err := os.Stat(store.jobDir(job.ID)); !errors.Is(err, os.ErrNotExist) {
+		t.Fatalf("failed reservation still exists: %v", err)
+	}
+}
+
 func TestStoreAppendRun(t *testing.T) {
 	s := newTestStore(t)
 	job, _ := s.Add(Job{Expr: "* * * * *", Prompt: "x", Status: StatusActive})

internal/dictation/dictation.go

@@ -37,16 +37,6 @@ const (
 	StreamProviderOpenAI   = "openai"
 )
 
-// BatchProviders lists valid stt.provider values, for config validation.
-func BatchProviders() []string {
-	return []string{ProviderLocal, ProviderGroq, ProviderOpenAI}
-}
-
-// StreamProviders lists valid stt.streamProvider values, for config validation.
-func StreamProviders() []string {
-	return []string{StreamProviderLocal, StreamProviderDeepgram, StreamProviderOpenAI}
-}
-
 // AudioFormat identifies a recorded container format, sniffed from the bytes —
 // desktop recorders produce WAV, Termux's recorder produces M4A/AAC.
 type AudioFormat string

internal/dictation/transcriber_deepgram.go

@@ -100,7 +100,7 @@ func (d *deepgramTranscriber) StreamTranscribe(ctx context.Context, chunks <-cha
 				}
 			default:
 			}
-			return compose(), fmt.Errorf("Deepgram stream error: %w", err)
+			return compose(), fmt.Errorf("Deepgram stream error: %w", err) //nolint:staticcheck // Preserve established user-facing error text.
 		}
 		if typ != websocket.MessageText {
 			continue

internal/imageinput/clipboard.go

@@ -2,6 +2,7 @@ package imageinput
 
 import (
 	"bytes"
+	"errors"
 	"fmt"
 	"net/http"
 	"os"
@@ -16,6 +17,13 @@ import (
 // clipboard, or (nil, "", nil) when the clipboard has no image. Called when
 // text clipboard is empty (the user pasted a screenshot). The media type is
 // sniffed from the bytes, not trusted from the clipboard.
+// ErrClipboardImageUnreadable reports that the clipboard holds an image the host
+// has no way to extract. It is deliberately distinct from "no image on the
+// clipboard", which is an ordinary no-op: this one is actionable, and silence is
+// the wrong response to it.
+var ErrClipboardImageUnreadable = errors.New(
+	"the clipboard holds an image but no helper on this host can read it; on macOS install pngpaste (brew install pngpaste) or a Python with pyobjc")
+
 func ReadClipboardImage() ([]byte, string, error) {
 	data, err := readClipboardImageBytes()
 	if err != nil {
@@ -95,35 +103,71 @@ func readClipboardImageWindows() ([]byte, error) {
 
 // readClipboardImageDarwin uses osascript to check for and read a clipboard
 // image. Returns (nil, nil) when no image is present.
-func readClipboardImageDarwin() ([]byte, error) {
-	// Check clipboard info for image classes.
-	check := `osascript -e 'clipboard info'`
-	out, err := exec.Command("sh", "-c", check).Output()
-	if err != nil {
-		return nil, nil
+// darwinClipboardInfo reports what the pasteboard is currently offering, and
+// darwinClipboardExtract pulls the PNG bytes out of it.
+//
+// Both are vars so the two-step logic below can be driven in a test. The step
+// that matters, "the clipboard holds an image and nothing here can extract it",
+// only happens when the first succeeds and the second fails, which cannot be
+// arranged by stubbing the caller: a test that substitutes ReadClipboardImage
+// asserts the plumbing around this function rather than the decision inside it,
+// and stays green with that decision reverted.
+var (
+	darwinClipboardInfo = func() (string, error) {
+		out, err := exec.Command("sh", "-c", `osascript -e 'clipboard info'`).Output()
+		return string(out), err
 	}
-	info := string(out)
-	if !strings.Contains(info, "PNG") && !strings.Contains(info, "JPEG") && !strings.Contains(info, "TIFF") && !strings.Contains(info, "GIF") {
-		return nil, nil
-	}
-	// Write clipboard image to a temp file via AppleScript, then read it.
-	// Using pngpaste if available, falling back to a Python one-liner.
-	cmd := exec.Command("sh", "-c", `pngpaste - 2>/dev/null || python3 -c "
+	darwinClipboardExtract = func() ([]byte, error) {
+		// pngpaste if available, falling back to a Python one-liner. Neither
+		// ships with macOS: pngpaste is Homebrew-only and Apple dropped the
+		// bundled PyObjC, so a stock Mac fails here every time.
+		cmd := exec.Command("sh", "-c", `pngpaste - 2>/dev/null || python3 -c "
 import AppKit, sys
 pb = AppKit.NSPasteboard.generalPasteboard()
 data = pb.dataForType_(AppKit.NSPasteboardTypePNG)
 if data:
     sys.stdout.buffer.write(data.bytes())
 "`)
-	var stdout bytes.Buffer
-	cmd.Stdout = &stdout
-	if err := cmd.Run(); err != nil {
+		var stdout bytes.Buffer
+		cmd.Stdout = &stdout
+		if err := cmd.Run(); err != nil {
+			return nil, err
+		}
+		return stdout.Bytes(), nil
+	}
+)
+
+// darwinClipboardOffersImage reports whether the pasteboard info line names an
+// image class.
+func darwinClipboardOffersImage(info string) bool {
+	for _, class := range []string{"PNG", "JPEG", "TIFF", "GIF"} {
+		if strings.Contains(info, class) {
+			return true
+		}
+	}
+	return false
+}
+
+func readClipboardImageDarwin() ([]byte, error) {
+	info, err := darwinClipboardInfo()
+	if err != nil {
 		return nil, nil
 	}
-	if stdout.Len() == 0 {
+	if !darwinClipboardOffersImage(info) {
 		return nil, nil
 	}
-	return stdout.Bytes(), nil
+	// Past this point the clipboard is KNOWN to hold an image, because the probe
+	// above said so. A failure here is therefore not "no image", it is "there is
+	// an image and nothing on this host can extract it". Reporting that as
+	// absence is what left users staring at a paste that silently did nothing.
+	data, err := darwinClipboardExtract()
+	if err != nil {
+		return nil, ErrClipboardImageUnreadable
+	}
+	if len(data) == 0 {
+		return nil, ErrClipboardImageUnreadable
+	}
+	return data, nil
 }
 
 // readClipboardImageLinux tries wl-paste (Wayland) then xclip (X11) to read

internal/imageinput/clipboard_darwin_logic_test.go

@@ -0,0 +1,84 @@
+package imageinput
+
+import (
+	"errors"
+	"testing"
+)
+
+// Drives readClipboardImageDarwin's own decision rather than the plumbing around
+// it. Both shell steps are substituted, so this runs on any platform and, more
+// importantly, actually reaches the branch under test: a test that stubs
+// ReadClipboardImage instead stays green with the production change reverted,
+// which is how the first version of this coverage slipped through.
+func TestReadClipboardImageDarwinDistinguishesUnreadableFromAbsent(t *testing.T) {
+	restore := func() func() {
+		info, extract := darwinClipboardInfo, darwinClipboardExtract
+		return func() { darwinClipboardInfo, darwinClipboardExtract = info, extract }
+	}()
+	t.Cleanup(restore)
+
+	const pngInfo = `«class PNGf», «class 8BPS»`
+
+	t.Run("image present but no extractor is actionable", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return nil, errors.New("exit status 127") }
+
+		data, err := readClipboardImageDarwin()
+		if !errors.Is(err, ErrClipboardImageUnreadable) {
+			t.Fatalf("err = %v, want ErrClipboardImageUnreadable; a stock Mac gets silence instead of a remedy", err)
+		}
+		if data != nil {
+			t.Fatalf("data = %q, want none", data)
+		}
+	})
+
+	t.Run("extractor producing nothing is also actionable", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return nil, nil }
+
+		if _, err := readClipboardImageDarwin(); !errors.Is(err, ErrClipboardImageUnreadable) {
+			t.Fatalf("err = %v, want ErrClipboardImageUnreadable for an extractor that exits 0 with no bytes", err)
+		}
+	})
+
+	// The other half of the distinction, and the reason this cannot simply always
+	// report an error: an empty clipboard is an ordinary no-op, and turning that
+	// into a notice would make every stray Ctrl+V nag the user.
+	t.Run("no image on the clipboard stays silent", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return `«class utf8»`, nil }
+		darwinClipboardExtract = func() ([]byte, error) {
+			t.Fatal("extractor ran despite the clipboard offering no image")
+			return nil, nil
+		}
+
+		data, err := readClipboardImageDarwin()
+		if err != nil || data != nil {
+			t.Fatalf("data=%q err=%v, want a silent no-op", data, err)
+		}
+	})
+
+	t.Run("probe failure stays silent", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return "", errors.New("osascript missing") }
+		darwinClipboardExtract = func() ([]byte, error) {
+			t.Fatal("extractor ran despite the info probe failing")
+			return nil, nil
+		}
+
+		if data, err := readClipboardImageDarwin(); err != nil || data != nil {
+			t.Fatalf("data=%q err=%v, want a silent no-op", data, err)
+		}
+	})
+
+	t.Run("working extractor returns the bytes", func(t *testing.T) {
+		darwinClipboardInfo = func() (string, error) { return pngInfo, nil }
+		darwinClipboardExtract = func() ([]byte, error) { return []byte("png-bytes"), nil }
+
+		data, err := readClipboardImageDarwin()
+		if err != nil {
+			t.Fatalf("err = %v, want none", err)
+		}
+		if string(data) != "png-bytes" {
+			t.Fatalf("data = %q, want the extractor's bytes", data)
+		}
+	})
+}

internal/imageinput/pdf_test.go

@@ -48,7 +48,7 @@ func buildMinimalPDF(text string) []byte {
 	buf.WriteString("0 " + strconv.Itoa(len(offsets)+1) + "\n")
 	buf.WriteString("0000000000 65535 f \n")
 	for _, off := range offsets {
-		buf.WriteString(fmt.Sprintf("%010d 00000 n \n", off))
+		fmt.Fprintf(&buf, "%010d 00000 n \n", off)
 	}
 	buf.WriteString("trailer\n<< /Size " + strconv.Itoa(len(offsets)+1) + " /Root 1 0 R >>\n")
 	buf.WriteString("startxref\n" + strconv.Itoa(xrefStart) + "\n%%EOF\n")
@@ -277,7 +277,7 @@ func buildEmptyTextPDF() []byte {
 	buf.WriteString("0 " + strconv.Itoa(len(offsets)+1) + "\n")
 	buf.WriteString("0000000000 65535 f \n")
 	for _, off := range offsets {
-		buf.WriteString(fmt.Sprintf("%010d 00000 n \n", off))
+		fmt.Fprintf(&buf, "%010d 00000 n \n", off)
 	}
 	buf.WriteString("trailer\n<< /Size " + strconv.Itoa(len(offsets)+1) + " /Root 1 0 R >>\n")
 	buf.WriteString("startxref\n" + strconv.Itoa(xrefStart) + "\n%%EOF\n")

internal/lsp/export_test.go

@@ -0,0 +1,14 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package lsp
+
+import "os/exec"
+
+// Available reports whether a configured server for the path exists on PATH.
+func Available(path string) bool {
+	cmd, ok := ServerFor(path)
+	if !ok {
+		return false
+	}
+	_, err := exec.LookPath(cmd[0])
+	return err == nil
+}

internal/lsp/navigate_test.go

@@ -1,6 +1,7 @@
 package lsp
 
 import (
+	"context"
 	"encoding/json"
 	"testing"
 )
@@ -64,7 +65,7 @@ func TestDecodeSymbols(t *testing.T) {
 
 func TestNavigateUnknownOp(t *testing.T) {
 	m := NewManager(t.TempDir())
-	_, _, ok, err := m.Navigate(nil, NavRequest{Op: "bogus", Path: "x.go"})
+	_, _, ok, err := m.Navigate(context.TODO(), NavRequest{Op: "bogus", Path: "x.go"})
 	if err == nil {
 		t.Fatal("expected an error for an unknown nav op")
 	}
@@ -76,7 +77,7 @@ func TestNavigateUnknownOp(t *testing.T) {
 func TestNavigateUnsupportedExtensionDegrades(t *testing.T) {
 	// A file type with no configured server degrades to ok=false, no error.
 	m := NewManager(t.TempDir())
-	_, _, ok, err := m.Navigate(nil, NavRequest{Op: NavDefinition, Path: "notes.unknownext", Line: 1, Character: 1})
+	_, _, ok, err := m.Navigate(context.TODO(), NavRequest{Op: NavDefinition, Path: "notes.unknownext", Line: 1, Character: 1})
 	if err != nil {
 		t.Fatalf("unsupported extension should not error, got %v", err)
 	}

internal/lsp/registry.go

@@ -1,7 +1,6 @@
 package lsp
 
 import (
-	"os/exec"
 	"path/filepath"
 	"sort"
 	"strings"
@@ -176,16 +175,6 @@ func LanguageID(path string) (string, bool) {
 	return id, ok
 }
 
-// Available reports whether a configured server for the path exists on PATH.
-func Available(path string) bool {
-	cmd, ok := ServerFor(path)
-	if !ok {
-		return false
-	}
-	_, err := exec.LookPath(cmd[0])
-	return err == nil
-}
-
 func extKey(path string) string {
 	return strings.ToLower(filepath.Ext(path))
 }

internal/mcp/registry_test.go

@@ -4,7 +4,6 @@ import (
 	"context"
 	"errors"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
@@ -289,7 +288,7 @@ func TestRegistryToolIsDeferredEligible(t *testing.T) {
 
 // TestRegistryToolReportsMCPServerName verifies the registryTool reports its true
 // configured server name (not the sanitized tool-name token) so the deferred-tools
-// reminder labels a multi-token server correctly via tools.DeferredLine.
+// discovery label names a multi-token server correctly via tools.DeferredSource.
 func TestRegistryToolReportsMCPServerName(t *testing.T) {
 	client := &fakeToolClient{}
 	// A server name that sanitizes to a token containing an underscore ("git_hub"):
@@ -305,14 +304,9 @@ func TestRegistryToolReportsMCPServerName(t *testing.T) {
 		t.Fatalf("MCPServerName() = %q, want %q", tool.MCPServerName(), "git hub")
 	}
 
-	// DeferredLine must prefer the reported server name over the name-derived token.
-	line := tools.DeferredLine(tool)
-	if !strings.Contains(line, "server: git hub") {
-		t.Fatalf("DeferredLine = %q, want it to label server as %q via MCPServerName()", line, "git hub")
-	}
-	// The truncated token-only label ("git") must NOT be the server segment.
-	if strings.Contains(line, "server: git |") {
-		t.Fatalf("DeferredLine = %q, mislabeled multi-token server with the truncated token", line)
+	// DeferredSource must prefer the reported server name over the name-derived token.
+	if source := tools.DeferredSource(tool); source != "git hub" {
+		t.Fatalf("DeferredSource = %q, want %q via MCPServerName()", source, "git hub")
 	}
 }
 

internal/minify/minify.go

@@ -13,8 +13,10 @@ package minify
 
 import (
 	"bytes"
+	"go/ast"
 	"go/parser"
 	"go/printer"
+	"go/scanner"
 	"go/token"
 	"path/filepath"
 	"strings"
@@ -34,8 +36,11 @@ func File(path string, content []byte) Result {
 		if out, ok := minifyGo(content); ok {
 			return Result{Content: out, Language: "go", Applied: true}
 		}
-		// Unparsable Go (a snippet, generics edge, or syntax error): fall through
-		// to the safe generic path rather than risk a partial AST reprint.
+		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.
 	} 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
@@ -46,6 +51,133 @@ 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,17 +39,58 @@ func Greet(name string) string {
 	}
 }
 
-func TestMinifyGoFallsBackOnUnparsableGo(t *testing.T) {
-	// A snippet (no package clause) cannot parse as a file -> safe generic path.
+func TestMinifyGoStatementFragment(t *testing.T) {
 	r := File("snippet.go", []byte("x := 1   \n\n\n// keep me\ny := 2\n"))
-	if r.Applied {
-		t.Fatalf("expected fallback for unparsable Go, got %+v", r)
+	if !r.Applied || r.Language != "go-fragment" {
+		t.Fatalf("expected parsed Go fragment, got %+v", r)
 	}
 	if strings.Contains(r.Content, "\n\n\n") {
-		t.Errorf("generic should collapse blank runs:\n%q", r.Content)
+		t.Errorf("fragment should collapse blank runs:\n%q", r.Content)
 	}
-	if !strings.Contains(r.Content, "// keep me") {
-		t.Errorf("generic must NOT strip comments (unsafe without a parser): %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)
 	}
 }
 

internal/modelregistry/export_test.go

@@ -0,0 +1,21 @@
+package modelregistry
+
+import "sync"
+
+// resetModelsDevCacheForTest clears the process-level cache memoization and
+// disables the overlay.
+func resetModelsDevCacheForTest() {
+	modelsDevOnce = sync.Once{}
+	modelsDevCached = nil
+	modelsDevEnabled.Store(false)
+}
+
+// Modes returns a copy of the preset catalog, preserving declaration order so
+// list output and help text stay stable.
+func Modes() []Mode {
+	modes := make([]Mode, len(defaultModes))
+	for index, mode := range defaultModes {
+		modes[index] = cloneMode(mode)
+	}
+	return modes
+}

internal/modelregistry/modelsdev.go

@@ -197,14 +197,6 @@ func cachedModelsDevProviders() map[string]map[string]modelsDevModel {
 	return modelsDevCached
 }
 
-// resetModelsDevCacheForTest clears the process-level cache memoization and
-// disables the overlay.
-func resetModelsDevCacheForTest() {
-	modelsDevOnce = sync.Once{}
-	modelsDevCached = nil
-	modelsDevEnabled.Store(false)
-}
-
 // RefreshModelsDevCache fetches models.dev/api.json into the on-disk cache
 // when the cache is missing or older than modelsDevRefreshAfter. It is safe to
 // call fire-and-forget from startup (use a goroutine); it never affects the

internal/modelregistry/modes.go

@@ -62,16 +62,6 @@ var defaultModes = []Mode{
 	},
 }
 
-// Modes returns a copy of the preset catalog, preserving declaration order so
-// list output and help text stay stable.
-func Modes() []Mode {
-	modes := make([]Mode, len(defaultModes))
-	for index, mode := range defaultModes {
-		modes[index] = cloneMode(mode)
-	}
-	return modes
-}
-
 // LookupMode returns the preset registered under name (case-insensitive,
 // whitespace-trimmed). The second result reports whether a preset matched.
 func LookupMode(name string) (Mode, bool) {

internal/notify/export_test.go

@@ -0,0 +1,7 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package notify
+
+// Enabled reports whether mode will ever emit a notification.
+func Enabled(mode Mode) bool {
+	return mode != "" && mode != ModeOff
+}

internal/notify/notify.go

@@ -167,11 +167,6 @@ func sequence(mode Mode, message string) string {
 	}
 }
 
-// Enabled reports whether mode will ever emit a notification.
-func Enabled(mode Mode) bool {
-	return mode != "" && mode != ModeOff
-}
-
 // sanitizeMessage drops control bytes (so the message can't break the escape or
 // inject terminal control) and clamps to maxMessageLen runes.
 func sanitizeMessage(s string) string {

internal/plugins/activate.go

@@ -284,7 +284,7 @@ const skillFileName = "SKILL.md"
 
 // MergedSkills loads the default skills directory, the shared ~/.agents/skills
 // root when present, plus the supplied plugin skill roots and returns one
-// merged, name-deduplicated list (Content stripped, like skills.List) alongside
+// merged, name-deduplicated list (Content stripped, like skills.ListFromRoots) alongside
 // the duplicate-name collisions across all roots. Earlier roots win a name
 // clash, matching skills.Load's first-wins rule; the default dir is always
 // considered first so a user skill shadows agents and plugins. A bad root simply

internal/providercatalog/catalog.go

@@ -138,6 +138,7 @@ var descriptors = []Descriptor{
 	openAICompat("groq", "Groq", "https://api.groq.com/openai/v1", "llama-3.3-70b-versatile", []string{"GROQ_API_KEY"}),
 	openAICompat("deepseek", "DeepSeek", "https://api.deepseek.com/v1", "deepseek-chat", []string{"DEEPSEEK_API_KEY"}),
 	openAICompat("together", "Together AI", "https://api.together.xyz/v1", "meta-llama/Llama-3.3-70B-Instruct-Turbo", []string{"TOGETHER_API_KEY"}),
+	openAICompat("fireworks", "Fireworks AI", "https://api.fireworks.ai/inference/v1", "accounts/fireworks/models/kimi-k2p7-code", []string{"FIREWORKS_API_KEY"}, "fireworks-ai"),
 	openAICompat("dashscope", "DashScope", "https://dashscope-intl.aliyuncs.com/compatible-mode/v1", "qwen-plus", []string{"DASHSCOPE_API_KEY", "QWEN_API_KEY"}, "qwen"),
 	openAICompat("moonshot", "Moonshot AI", "https://api.moonshot.ai/v1", "kimi-k2-0905-preview", []string{"MOONSHOT_API_KEY"}, "kimi"),
 	openAICompat("atlascloud", "Atlas Cloud", "https://api.atlascloud.ai/v1", "qwen/qwen3.5-flash", []string{"ATLASCLOUD_API_KEY"}, "atlas cloud", "atlas"),
@@ -187,14 +188,6 @@ func All() []Descriptor {
 	return copied
 }
 
-func IDs() []string {
-	ids := make([]string, 0, len(descriptors))
-	for _, descriptor := range descriptors {
-		ids = append(ids, descriptor.ID)
-	}
-	return ids
-}
-
 func Get(id string) (Descriptor, bool) {
 	normalized := NormalizeID(id)
 	for _, descriptor := range descriptors {
@@ -219,35 +212,6 @@ func Require(id string) (Descriptor, error) {
 	return descriptor, nil
 }
 
-func ListByTransport(transport Transport) []Descriptor {
-	normalized := Transport(NormalizeID(string(transport)))
-	items := make([]Descriptor, 0)
-	for _, descriptor := range descriptors {
-		if descriptor.Transport == normalized {
-			items = append(items, cloneDescriptor(descriptor))
-		}
-	}
-	return items
-}
-
-func ValidTransport(transport Transport) bool {
-	switch Transport(NormalizeID(string(transport))) {
-	case TransportOpenAI, TransportAnthropic, TransportGoogle, TransportBedrock, TransportVertex, TransportOpenAICompatible, TransportAnthropicCompatible:
-		return true
-	default:
-		return false
-	}
-}
-
-func ValidAPIFormat(format APIFormat) bool {
-	switch format {
-	case APIFormatOpenAIResponses, APIFormatOpenAIChatCompletions, APIFormatAnthropicMessages, APIFormatGoogleGenerateContent, APIFormatBedrockConverse, APIFormatVertexGenerateContent:
-		return true
-	default:
-		return false
-	}
-}
-
 func NormalizeID(id string) string {
 	var builder strings.Builder
 	lastDash := false

internal/providercatalog/catalog_test.go

@@ -22,6 +22,7 @@ var expectedCatalogIDs = []string{
 	"groq",
 	"deepseek",
 	"together",
+	"fireworks",
 	"dashscope",
 	"moonshot",
 	"atlascloud",
@@ -155,6 +156,28 @@ func TestLongCatDescriptor(t *testing.T) {
 	}
 }
 
+func TestFireworksDescriptor(t *testing.T) {
+	descriptor, err := Require("fireworks")
+	if err != nil {
+		t.Fatalf("Require(fireworks) error = %v", err)
+	}
+	if descriptor.Name != "Fireworks AI" {
+		t.Fatalf("Name = %q, want Fireworks AI", descriptor.Name)
+	}
+	if descriptor.DefaultBaseURL != "https://api.fireworks.ai/inference/v1" {
+		t.Fatalf("DefaultBaseURL = %q, want Fireworks OpenAI-compatible endpoint", descriptor.DefaultBaseURL)
+	}
+	if descriptor.DefaultModel != "accounts/fireworks/models/kimi-k2p7-code" {
+		t.Fatalf("DefaultModel = %q, want accounts/fireworks/models/kimi-k2p7-code", descriptor.DefaultModel)
+	}
+	if descriptor.Transport != TransportOpenAICompatible {
+		t.Fatalf("Transport = %q, want %q", descriptor.Transport, TransportOpenAICompatible)
+	}
+	if !reflect.DeepEqual(descriptor.AuthEnvVars, []string{"FIREWORKS_API_KEY"}) {
+		t.Fatalf("AuthEnvVars = %#v, want FIREWORKS_API_KEY", descriptor.AuthEnvVars)
+	}
+}
+
 func TestAtlasCloudDescriptor(t *testing.T) {
 	descriptor, err := Require("atlascloud")
 	if err != nil {
@@ -346,7 +369,7 @@ func TestListByTransportPreservesCatalogOrder(t *testing.T) {
 		TransportBedrock:         {"bedrock"},
 		TransportVertex:          {"vertex"},
 		TransportAnthropicCompat: {"minimax", "minimaxi-cn", "opencode-go-anthropic-compatible", "custom-anthropic-compatible"},
-		TransportOpenAICompat:    {"gitlawb-opengateway", "aimlapi", "ollama-cloud", "ollama", "lmstudio", "openrouter", "huggingface", "chatgpt", "groq", "deepseek", "together", "dashscope", "moonshot", "atlascloud", "longcat", "nvidia-nim", "mistral", "github", "xai", "venice", "xiaomi-mimo", "bankr", "zai", "zai-cn", "kilocode", "opencode", "opencode-go", "atomic-chat", "chatgpt-proxy", "custom-openai-compatible"},
+		TransportOpenAICompat:    {"gitlawb-opengateway", "aimlapi", "ollama-cloud", "ollama", "lmstudio", "openrouter", "huggingface", "chatgpt", "groq", "deepseek", "together", "fireworks", "dashscope", "moonshot", "atlascloud", "longcat", "nvidia-nim", "mistral", "github", "xai", "venice", "xiaomi-mimo", "bankr", "zai", "zai-cn", "kilocode", "opencode", "opencode-go", "atomic-chat", "chatgpt-proxy", "custom-openai-compatible"},
 	}
 
 	for transport, wantIDs := range cases {

internal/providercatalog/export_test.go

@@ -0,0 +1,39 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package providercatalog
+
+func IDs() []string {
+	ids := make([]string, 0, len(descriptors))
+	for _, descriptor := range descriptors {
+		ids = append(ids, descriptor.ID)
+	}
+	return ids
+}
+
+func ListByTransport(transport Transport) []Descriptor {
+	normalized := Transport(NormalizeID(string(transport)))
+	items := make([]Descriptor, 0)
+	for _, descriptor := range descriptors {
+		if descriptor.Transport == normalized {
+			items = append(items, cloneDescriptor(descriptor))
+		}
+	}
+	return items
+}
+
+func ValidAPIFormat(format APIFormat) bool {
+	switch format {
+	case APIFormatOpenAIResponses, APIFormatOpenAIChatCompletions, APIFormatAnthropicMessages, APIFormatGoogleGenerateContent, APIFormatBedrockConverse, APIFormatVertexGenerateContent:
+		return true
+	default:
+		return false
+	}
+}
+
+func ValidTransport(transport Transport) bool {
+	switch Transport(NormalizeID(string(transport))) {
+	case TransportOpenAI, TransportAnthropic, TransportGoogle, TransportBedrock, TransportVertex, TransportOpenAICompatible, TransportAnthropicCompatible:
+		return true
+	default:
+		return false
+	}
+}

internal/providerhealth/providerhealth.go

@@ -46,12 +46,13 @@ const (
 const defaultTimeout = 5 * time.Second
 
 type Options struct {
-	Profile      config.ProviderProfile
-	Connectivity bool
-	HTTPClient   *http.Client
-	Resolver     Resolver
-	Timeout      time.Duration
-	UserAgent    string
+	Profile       config.ProviderProfile
+	Connectivity  bool
+	HTTPClient    *http.Client
+	Resolver      Resolver
+	OAuthResolver providerio.TokenResolver
+	Timeout       time.Duration
+	UserAgent     string
 }
 
 type Resolver interface {
@@ -207,11 +208,26 @@ func Probe(ctx context.Context, options Options) Result {
 		"providerKind": metadata.ProviderKind,
 	}, profile))
 
-	if credentialRequired(profile, metadata.ProviderKind) && !hasCredential(profile) {
+	oauthConfigured := options.OAuthResolver != nil
+	if oauthConfigured {
+		var resolveErr error
+		profile, oauthConfigured, resolveErr = profileWithOAuthCredential(ctx, profile, options.OAuthResolver, false)
+		if resolveErr != nil {
+			result.add(check("provider.auth", "Provider auth", StatusFail, CategoryAuth, "Provider OAuth credential could not be resolved: "+resolveErr.Error(), map[string]any{
+				"oauthLogin": true,
+			}, profile))
+			return result.finalize()
+		}
+	}
+	if credentialRequired(profile, metadata.ProviderKind) && !hasCredential(profile) && !oauthConfigured {
 		result.add(check("provider.auth", "Provider auth", StatusFail, CategoryAuth, fmt.Sprintf("Provider %s requires API credentials.", providerName(profile)), credentialDetails(profile), profile))
 		return result.finalize()
 	}
-	if hasCredential(profile) {
+	if oauthConfigured {
+		result.add(check("provider.auth", "Provider auth", StatusPass, CategoryAuth, fmt.Sprintf("Provider %s has OAuth credentials configured.", providerName(profile)), map[string]any{
+			"oauthLogin": true,
+		}, profile))
+	} else if hasCredential(profile) {
 		result.add(check("provider.auth", "Provider auth", StatusPass, CategoryAuth, fmt.Sprintf("Provider %s has credentials configured.", providerName(profile)), credentialDetails(profile), profile))
 	} else {
 		result.add(check("provider.auth", "Provider auth", StatusPass, CategoryAuth, fmt.Sprintf("Provider %s does not require API credentials.", providerName(profile)), credentialDetails(profile), profile))
@@ -286,9 +302,35 @@ func connectivityCheck(ctx context.Context, profile config.ProviderProfile, kind
 	if err != nil {
 		return classifyTransportError(err, profile)
 	}
-	defer func() {
-		_ = response.Body.Close()
-	}()
+	defer func(body io.ReadCloser) {
+		_ = body.Close()
+	}(response.Body)
+	if response.StatusCode == http.StatusUnauthorized && options.OAuthResolver != nil {
+		refreshed, ok, refreshErr := profileWithOAuthCredential(requestCtx, options.Profile, options.OAuthResolver, true)
+		if refreshErr != nil {
+			return check("provider.connectivity", "Provider connectivity", StatusFail, CategoryAuth, "Provider OAuth credential refresh failed: "+refreshErr.Error(), map[string]any{
+				"statusCode": response.StatusCode,
+			}, profile)
+		}
+		if ok {
+			_ = response.Body.Close()
+			profile = refreshed
+			request, allowLoopbackOrPrivate, err = healthRequest(requestCtx, profile, kind, options)
+			if err != nil {
+				return check("provider.connectivity", "Provider connectivity", StatusFail, CategoryConfig, err.Error(), nil, profile)
+			}
+			if options.HTTPClient == nil {
+				client = newConnectivityClient(timeout, options.Resolver, sensitiveAuthHeaderNames(profile, kind), allowLoopbackOrPrivate)
+			}
+			response, err = client.Do(request)
+			if err != nil {
+				return classifyTransportError(err, profile)
+			}
+			defer func(body io.ReadCloser) {
+				_ = body.Close()
+			}(response.Body)
+		}
+	}
 
 	body, err := io.ReadAll(io.LimitReader(response.Body, 64*1024))
 	if err != nil {
@@ -320,6 +362,23 @@ func connectivityCheck(ctx context.Context, profile config.ProviderProfile, kind
 	}, profile)
 }
 
+func profileWithOAuthCredential(ctx context.Context, profile config.ProviderProfile, resolver providerio.TokenResolver, forceRefresh bool) (config.ProviderProfile, bool, error) {
+	if resolver == nil {
+		return profile, false, nil
+	}
+	header, value, ok, err := resolver(ctx, forceRefresh)
+	if err != nil || !ok {
+		return profile, false, err
+	}
+	if strings.TrimSpace(header) == "" {
+		header = "Authorization"
+	}
+	profile.AuthHeader = header
+	profile.AuthScheme = ""
+	profile.AuthHeaderValue = value
+	return profile, true, nil
+}
+
 func healthRequest(ctx context.Context, profile config.ProviderProfile, kind config.ProviderKind, options Options) (*http.Request, bool, error) {
 	baseURL, err := resolvedBaseURL(profile, kind)
 	if err != nil {
@@ -391,7 +450,7 @@ func validateEndpoint(ctx context.Context, endpoint string, resolver Resolver, a
 		return endpointSafetyError{message: "provider connectivity URL is unsafe: localhost hosts are blocked"}
 	}
 	if addr, err := netip.ParseAddr(normalized); err == nil {
-		if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
+		if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
 			return endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
 		}
 		return nil
@@ -407,7 +466,7 @@ func validateEndpoint(ctx context.Context, endpoint string, resolver Resolver, a
 		return endpointSafetyError{message: "provider connectivity host resolved to no addresses"}
 	}
 	for _, addr := range addrs {
-		if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
+		if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
 			return endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
 		}
 	}
@@ -529,7 +588,7 @@ func safeDialContext(resolver Resolver, allowLoopbackOrPrivate bool) func(contex
 			return nil, err
 		}
 		if addr, parseErr := netip.ParseAddr(host); parseErr == nil {
-			if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
+			if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
 				return nil, endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
 			}
 			return dialer.DialContext(ctx, network, address)
@@ -542,7 +601,7 @@ func safeDialContext(resolver Resolver, allowLoopbackOrPrivate bool) func(contex
 			return nil, endpointSafetyError{message: "provider connectivity host resolved to no addresses"}
 		}
 		for _, addr := range addrs {
-			if reason := blockedAddrReason(addr); reason != "" && !(allowLoopbackOrPrivate && (addr.IsLoopback() || addr.IsPrivate())) {
+			if reason := blockedAddrReason(addr); reason != "" && (!allowLoopbackOrPrivate || (!addr.IsLoopback() && !addr.IsPrivate())) {
 				return nil, endpointSafetyError{message: "provider connectivity URL is unsafe: " + reason}
 			}
 		}
@@ -748,6 +807,9 @@ func redact(message string, profile config.ProviderProfile) string {
 
 func providerSecrets(profile config.ProviderProfile) []string {
 	secrets := []string{profile.APIKey, profile.AuthHeaderValue}
+	if fields := strings.Fields(profile.AuthHeaderValue); len(fields) == 2 {
+		secrets = append(secrets, fields[1])
+	}
 	for _, value := range profile.CustomHeaders {
 		if strings.TrimSpace(value) != "" {
 			secrets = append(secrets, value)

internal/providerhealth/providerhealth_test.go

@@ -180,6 +180,182 @@ func TestProbeResolvedKindConnectivityAuthErrorRedactsSecret(t *testing.T) {
 	}
 }
 
+func TestProbeConnectivityUsesOAuthCredential(t *testing.T) {
+	var gotAuth string
+	resolverCalled := false
+	client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+		gotAuth = r.Header.Get("Authorization")
+		return &http.Response{
+			StatusCode: http.StatusOK,
+			Status:     "200 OK",
+			Body:       io.NopCloser(strings.NewReader(`{"data":[]}`)),
+			Header:     make(http.Header),
+		}, nil
+	})}
+
+	result := Probe(context.Background(), Options{
+		Profile: config.ProviderProfile{
+			Name:         "xai",
+			CatalogID:    "xai",
+			ProviderKind: config.ProviderKindOpenAICompatible,
+			BaseURL:      "https://api.example.com/v1",
+			Model:        "grok-4",
+		},
+		Connectivity: true,
+		HTTPClient:   client,
+		Resolver:     staticResolver{addr: netip.MustParseAddr("93.184.216.34")},
+		OAuthResolver: func(context.Context, bool) (string, string, bool, error) {
+			resolverCalled = true
+			return "Authorization", "Bearer oauth-test-token", true, nil
+		},
+	})
+
+	if result.Status != StatusPass {
+		t.Fatalf("Status = %q, want pass: %#v", result.Status, result.Checks)
+	}
+	if !resolverCalled {
+		t.Fatal("OAuth resolver was not called")
+	}
+	if gotAuth != "Bearer oauth-test-token" {
+		t.Fatalf("Authorization = %q, want OAuth bearer", gotAuth)
+	}
+}
+
+func TestProbeConnectivityOAuthCredentialsAreRedacted(t *testing.T) {
+	const (
+		staticKey  = "STATICKEYVALUE1234567890abcdef"
+		oauthToken = "hf_VASANTHSENTINEL0011223344"
+	)
+	var authHeaders []string
+	client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+		authHeaders = append(authHeaders, r.Header.Get("Authorization"))
+		return &http.Response{
+			StatusCode: http.StatusUnauthorized,
+			Status:     "401 Unauthorized",
+			Body:       io.NopCloser(strings.NewReader(`{"error":{"message":"invalid credentials ` + staticKey + ` and ` + oauthToken + `"}}`)),
+			Header:     make(http.Header),
+		}, nil
+	})}
+
+	result := Probe(context.Background(), Options{
+		Profile: config.ProviderProfile{
+			Name:         "xai",
+			CatalogID:    "xai",
+			ProviderKind: config.ProviderKindOpenAICompatible,
+			BaseURL:      "https://api.example.com/v1",
+			APIKey:       staticKey,
+			Model:        "grok-4",
+		},
+		Connectivity: true,
+		HTTPClient:   client,
+		Resolver:     staticResolver{addr: netip.MustParseAddr("93.184.216.34")},
+		OAuthResolver: func(context.Context, bool) (string, string, bool, error) {
+			return "Authorization", "Bearer " + oauthToken, true, nil
+		},
+	})
+
+	if len(authHeaders) != 2 {
+		t.Fatalf("Authorization headers = %#v, want initial request and one refresh retry", authHeaders)
+	}
+	for _, header := range authHeaders {
+		if header != "Bearer "+oauthToken {
+			t.Fatalf("Authorization = %q, want OAuth bearer", header)
+		}
+	}
+	check := result.Check("provider.connectivity")
+	if check == nil || check.Category != CategoryAuth {
+		t.Fatalf("provider.connectivity = %#v, want auth failure", check)
+	}
+	if strings.Contains(check.Message, staticKey) {
+		t.Fatalf("static API key leaked in message: %q", check.Message)
+	}
+	if strings.Contains(check.Message, oauthToken) {
+		t.Fatalf("OAuth token leaked in message: %q", check.Message)
+	}
+	if strings.Count(check.Message, "[REDACTED]") != 2 {
+		t.Fatalf("expected both credentials to be redacted, got %q", check.Message)
+	}
+}
+
+func TestProbeWithoutConnectivityRejectsUnavailableOAuthCredential(t *testing.T) {
+	resolverCalled := false
+	result := Probe(context.Background(), Options{
+		Profile: config.ProviderProfile{
+			Name:         "xai",
+			CatalogID:    "xai",
+			ProviderKind: config.ProviderKindOpenAICompatible,
+			BaseURL:      "https://api.example.com/v1",
+			Model:        "grok-4",
+		},
+		OAuthResolver: func(context.Context, bool) (string, string, bool, error) {
+			resolverCalled = true
+			return "", "", false, nil
+		},
+	})
+
+	if !resolverCalled {
+		t.Fatal("OAuth resolver was not called")
+	}
+	if result.Status != StatusFail {
+		t.Fatalf("Status = %q, want fail: %#v", result.Status, result.Checks)
+	}
+	check := result.Check("provider.auth")
+	if check == nil || check.Status != StatusFail || check.Category != CategoryAuth {
+		t.Fatalf("provider.auth = %#v, want auth failure", check)
+	}
+}
+
+func TestProbeConnectivityRefreshesOAuthCredentialAfterUnauthorized(t *testing.T) {
+	var authHeaders []string
+	client := &http.Client{Transport: roundTripFunc(func(r *http.Request) (*http.Response, error) {
+		authHeaders = append(authHeaders, r.Header.Get("Authorization"))
+		status := http.StatusOK
+		body := `{"data":[]}`
+		if len(authHeaders) == 1 {
+			status = http.StatusUnauthorized
+			body = `{"error":{"message":"expired"}}`
+		}
+		return &http.Response{
+			StatusCode: status,
+			Status:     http.StatusText(status),
+			Body:       io.NopCloser(strings.NewReader(body)),
+			Header:     make(http.Header),
+		}, nil
+	})}
+
+	var refreshFlags []bool
+	result := Probe(context.Background(), Options{
+		Profile: config.ProviderProfile{
+			Name:         "xai",
+			CatalogID:    "xai",
+			ProviderKind: config.ProviderKindOpenAICompatible,
+			BaseURL:      "https://api.example.com/v1",
+			Model:        "grok-4",
+		},
+		Connectivity: true,
+		HTTPClient:   client,
+		Resolver:     staticResolver{addr: netip.MustParseAddr("93.184.216.34")},
+		OAuthResolver: func(_ context.Context, forceRefresh bool) (string, string, bool, error) {
+			refreshFlags = append(refreshFlags, forceRefresh)
+			token := "stale-token"
+			if forceRefresh {
+				token = "fresh-token"
+			}
+			return "Authorization", "Bearer " + token, true, nil
+		},
+	})
+
+	if result.Status != StatusPass {
+		t.Fatalf("Status = %q, want pass after refresh: %#v", result.Status, result.Checks)
+	}
+	if len(refreshFlags) != 2 || refreshFlags[0] || !refreshFlags[1] {
+		t.Fatalf("OAuth refresh flags = %#v, want [false true]", refreshFlags)
+	}
+	if len(authHeaders) != 2 || authHeaders[0] != "Bearer stale-token" || authHeaders[1] != "Bearer fresh-token" {
+		t.Fatalf("Authorization headers = %#v, want stale then fresh bearer", authHeaders)
+	}
+}
+
 func TestProbeConnectivityClassifiesTimeout(t *testing.T) {
 	client := &http.Client{Transport: roundTripFunc(func(*http.Request) (*http.Response, error) {
 		return nil, context.DeadlineExceeded

internal/providermodelcatalog/catalog.go

@@ -122,6 +122,14 @@ var curatedModels = map[string][]Model{
 		{ID: "deepseek-ai/DeepSeek-R1", Description: "reasoning model"},
 		{ID: "meta-llama/Llama-4-Maverick-17B-128E-Instruct-FP8", Description: "multimodal model"},
 	},
+	"fireworks": {
+		// Serverless-only curated set: ordinary API-key setups hit /inference/v1
+		// and cannot use on-demand deployment IDs.
+		{ID: "accounts/fireworks/models/kimi-k2p7-code", Description: "catalog default"},
+		{ID: "accounts/fireworks/models/deepseek-v4-flash", Description: "fast coding model"},
+		{ID: "accounts/fireworks/models/gpt-oss-120b", Description: "general model"},
+		{ID: "accounts/fireworks/models/deepseek-v4-pro", Description: "reasoning model"},
+	},
 	"dashscope": {
 		{ID: "qwen-plus", Description: "catalog default"},
 		{ID: "qwen-max", Description: "strong general model"},

internal/providermodelcatalog/catalog_test.go

@@ -28,6 +28,11 @@ func TestModelsAreProviderScoped(t *testing.T) {
 			want:     []string{"llama-3.3-70b-versatile", "openai/gpt-oss-120b"},
 			notWant:  []string{"gpt-4.1", "claude-sonnet-4.5"},
 		},
+		{
+			provider: "fireworks",
+			want:     []string{"accounts/fireworks/models/kimi-k2p7-code", "accounts/fireworks/models/deepseek-v4-flash"},
+			notWant:  []string{"gpt-4.1", "llama-3.3-70b-versatile"},
+		},
 		{
 			provider: "chatgpt",
 			want:     []string{"gpt-5.5", "gpt-5.4", "gpt-5.4-mini", "gpt-5.3-codex-spark"},