fix(config): make the provider-command timeout an upper bound (#811)

providerCommandTimeout bounded only the wait, so the call could take arbitrarily
longer than the 5s its error text promised. Two phases sat outside it.

Starting the process ran before the clock began. On Windows that is
CREATE_SUSPENDED, job-object creation and assignment, and a system-wide thread
snapshot, none of it fast on a cold or contended machine. The deadline is now
armed first and the start races it, so process creation is inside the budget.

Draining after termination was unbounded too: the timer path called Terminate
and then received on the wait with no limit. Terminate normally makes that
immediate, which is exactly why it survived unnoticed, but a tree that resists
termination held the call open long past the deadline that had just expired.
That receive is now bounded, and a call that gives up leaves cmd.Wait running,
so the output buffers are synchronised: the I/O pump keeps writing into buffers
the caller already holds, which a plain bytes.Buffer cannot survive.

The timeout error also carried nothing actionable. It was 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.
It now names the phase, wraps the cause, and includes the redacted output the
failure path already had.

Both bounds are asserted on extracted helpers rather than end to end, because
end to end they are untestable: a terminated command stops writing and its wait
returns at once, so an unbounded drain and an unsynchronised buffer both pass a
whole-call test. The first version of these tests did exactly that and proved
nothing. Removing either bound now fails, as does arming the deadline after the
start, which reports the wrong phase.

.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

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).
 

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

@@ -367,22 +367,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/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

@@ -16,7 +16,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 +51,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 +88,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(),
@@ -117,7 +117,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(

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_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,
@@ -427,7 +427,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 +491,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"})
 

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
@@ -2866,19 +2862,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.

internal/agent/loop_test.go

@@ -863,9 +863,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 +880,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 +957,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 +1164,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 +1194,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 +1231,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 +1269,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 +1353,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 +1421,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 +1468,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 +1544,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 +1601,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 +1644,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 +1700,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 +1749,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 +1797,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 +1893,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 +1965,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 +2046,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 +2248,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 +2355,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{
 			{
@@ -2419,7 +2416,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 +2483,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{
 			{
@@ -2529,7 +2526,7 @@ func TestRunDoesNotOfferPrefixApprovalForUnsafeBashCommand(t *testing.T) {
 func TestRunPromptsForDestructiveShellInsteadOfSandboxDeny(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{
 			{
@@ -2598,7 +2595,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
 
@@ -2662,7 +2659,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 +2711,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 +2890,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 +2939,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 +2969,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{
 			{
@@ -3231,7 +3228,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 +3265,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 +3298,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 +3439,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/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/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/types.go

@@ -427,13 +427,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/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/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/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

@@ -734,14 +734,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/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

@@ -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

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

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

@@ -187,14 +187,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 +211,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/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

@@ -391,7 +391,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 +407,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 +529,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 +542,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}
 			}
 		}

internal/provideronboarding/localruntime_test.go

@@ -62,7 +62,7 @@ func TestDetectLocalRuntimesReportsReachableRuntime(t *testing.T) {
 	if !detected[0].Reachable {
 		t.Fatalf("runtime should be reachable: %#v", detected[0])
 	}
-	if detected[0].Models == nil || len(detected[0].Models) == 0 || detected[0].Models[0] != "llama3.1" {
+	if len(detected[0].Models) == 0 || detected[0].Models[0] != "llama3.1" {
 		t.Fatalf("expected discovered model list, got %#v", detected[0].Models)
 	}
 }

internal/providers/openai/codex.go

@@ -3,7 +3,6 @@ package openai
 import (
 	"context"
 	"encoding/json"
-	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -207,14 +206,3 @@ func (p *CodexProvider) resolveAccount(ctx context.Context) (string, bool, error
 	}
 	return "", false, nil
 }
-
-// ValidateAccount is a convenience for tests/callers that want to confirm the
-// account id is the right shape (non-empty, trimmed). It is a no-op helper
-// rather than a constructor check so a Codex provider can be built before the
-// first login completes.
-func ValidateAccount(account string) error {
-	if strings.TrimSpace(account) == "" {
-		return errors.New("openai codex: account id is empty")
-	}
-	return nil
-}

internal/providers/openai/export_test.go

@@ -0,0 +1,18 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package openai
+
+import (
+	"errors"
+	"strings"
+)
+
+// ValidateAccount is a convenience for tests/callers that want to confirm the
+// account id is the right shape (non-empty, trimmed). It is a no-op helper
+// rather than a constructor check so a Codex provider can be built before the
+// first login completes.
+func ValidateAccount(account string) error {
+	if strings.TrimSpace(account) == "" {
+		return errors.New("openai codex: account id is empty")
+	}
+	return nil
+}

internal/providers/openai/provider_test.go

@@ -945,9 +945,9 @@ func TestStreamCompletionSurfacesLengthFinishReason(t *testing.T) {
 		t.Fatalf("done FinishReason = %q, want %q", doneReason, zeroruntime.FinishReasonLength)
 	}
 
-	// And it round-trips through the runtime collector as Truncated.
+	// And it round-trips through the runtime collector's FinishReason.
 	collected := zeroruntime.CollectStream(context.Background(), replay(events))
-	if !collected.Truncated() || collected.FinishReason != zeroruntime.FinishReasonLength {
+	if collected.FinishReason != zeroruntime.FinishReasonLength {
 		t.Fatalf("collected = %+v, want truncated length", collected)
 	}
 }

internal/providers/providerio/export_test.go

@@ -0,0 +1,14 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package providerio
+
+import (
+	"bufio"
+	"io"
+)
+
+// ScanSSEData parses Server-Sent Event data fields from a streaming response.
+func ScanSSEData(reader io.Reader, handle func(data string) bool) error {
+	scanner := bufio.NewScanner(reader)
+	scanner.Buffer(make([]byte, 0, 4096), maxSSELineBytes)
+	return scanSSEPayloads(scanner, handle, nil)
+}

internal/providers/providerio/providerio.go

@@ -195,13 +195,6 @@ func SendEvent(ctx context.Context, events chan<- zeroruntime.StreamEvent, event
 	}
 }
 
-// ScanSSEData parses Server-Sent Event data fields from a streaming response.
-func ScanSSEData(reader io.Reader, handle func(data string) bool) error {
-	scanner := bufio.NewScanner(reader)
-	scanner.Buffer(make([]byte, 0, 4096), maxSSELineBytes)
-	return scanSSEPayloads(scanner, handle, nil)
-}
-
 // scanSSEPayloads accumulates SSE "data:" lines into payloads (joined across
 // continuation lines, flushed on a blank line or EOF) and forwards each to
 // handle. It is the shared core of ScanSSEData and the idle-aware variant.

internal/reasoning/capability.go

@@ -1,147 +0,0 @@
-// Package reasoning models how each provider's models expose reasoning-effort
-// control. Providers disagree on the concept — OpenAI uses a discrete effort
-// enum, Anthropic and Gemini 2.5 use a thinking-token budget, some models only
-// toggle thinking on/off — so a single flat effort string cannot describe them.
-//
-// Capability is the typed, per-model description of that control, sourced from a
-// community capability catalog (models.dev). It is the data the rest of Zero
-// consults to decide which reasoning tiers a model actually supports, replacing
-// model-name guessing. This package depends only on the standard library, so the
-// model registry and provider adapters can import it without a cycle.
-//
-// Source of truth: this catalog is intended to become the authoritative reasoning
-// -capability source. When it is wired into the live path, modelregistry's
-// name-pattern inference (reasoningEffortsForModelName) is demoted to a fallback
-// for models the catalog does not cover; the catalog wins where it has an entry.
-// This commit is additive groundwork — nothing consumes it yet — so there is no
-// behavioral divergence until that wiring lands.
-package reasoning
-
-import "strings"
-
-// ControlKind enumerates how a model exposes reasoning control. The values
-// mirror the models.dev reasoning_options[].type field.
-type ControlKind string
-
-const (
-	// ControlEffort is a discrete effort enum (OpenAI reasoning_effort, Gemini 3
-	// thinkingLevel, newer Claude output_config.effort).
-	ControlEffort ControlKind = "effort"
-	// ControlBudget is a thinking-token budget (Gemini 2.5 thinkingBudget, legacy
-	// Claude thinking.budget_tokens).
-	ControlBudget ControlKind = "budget_tokens"
-	// ControlToggle is an on/off thinking switch with no levels.
-	ControlToggle ControlKind = "toggle"
-)
-
-// Control is one reasoning control a model accepts. For an effort control,
-// Values lists the accepted tiers ordered weakest to strongest. For a budget
-// control, Min/Max bound the thinking-token budget; each is nil when the
-// provider does not bound that side, kept distinct from an explicit 0 (Gemini
-// uses min: 0 to mean "thinking can be disabled", which a plain int would lose).
-type Control struct {
-	Kind   ControlKind `json:"type"`
-	Values []string    `json:"values,omitempty"`
-	Min    *int        `json:"min,omitempty"`
-	Max    *int        `json:"max,omitempty"`
-}
-
-// clone returns a deep copy of the control so the embedded catalog cannot be
-// mutated through a returned Control's slice or budget pointers.
-func (ctrl Control) clone() Control {
-	out := Control{Kind: ctrl.Kind}
-	if ctrl.Values != nil {
-		out.Values = append([]string(nil), ctrl.Values...)
-	}
-	if ctrl.Min != nil {
-		min := *ctrl.Min
-		out.Min = &min
-	}
-	if ctrl.Max != nil {
-		max := *ctrl.Max
-		out.Max = &max
-	}
-	return out
-}
-
-// Capability is the reasoning capability of a single model: whether it reasons
-// at all, and through which controls. A model may carry more than one control
-// (e.g. a newer Claude model exposes both an effort enum and a token budget); a
-// reasoning model with no controls reasons but exposes no knob (always-on).
-type Capability struct {
-	Reasoning bool      `json:"reasoning"`
-	Controls  []Control `json:"reasoning_options,omitempty"`
-}
-
-// clone returns a deep copy of the capability, cloning each control, so a caller
-// cannot mutate the shared catalog through the returned value's slices.
-func (c Capability) clone() Capability {
-	out := Capability{Reasoning: c.Reasoning}
-	if c.Controls != nil {
-		out.Controls = make([]Control, len(c.Controls))
-		for i, ctrl := range c.Controls {
-			out.Controls[i] = ctrl.clone()
-		}
-	}
-	return out
-}
-
-// Supported reports whether the model performs any reasoning.
-func (c Capability) Supported() bool { return c.Reasoning }
-
-// EffortControl returns the model's effort control and whether it has one. The
-// returned Control is a deep copy, so a caller cannot mutate the shared catalog
-// through its Values slice.
-func (c Capability) EffortControl() (Control, bool) {
-	for _, ctrl := range c.Controls {
-		if ctrl.Kind == ControlEffort {
-			return ctrl.clone(), true
-		}
-	}
-	return Control{}, false
-}
-
-// EffortValues returns the ordered effort tiers the model accepts, or nil when
-// it has no effort control (a budget- or toggle-only model, or a non-reasoning
-// model).
-func (c Capability) EffortValues() []string {
-	if ctrl, ok := c.EffortControl(); ok {
-		return ctrl.Values
-	}
-	return nil
-}
-
-// SupportsEffort reports whether tier is one of the model's accepted effort
-// values (case-insensitive).
-func (c Capability) SupportsEffort(tier string) bool {
-	tier = strings.ToLower(strings.TrimSpace(tier))
-	for _, v := range c.EffortValues() {
-		if strings.ToLower(v) == tier {
-			return true
-		}
-	}
-	return false
-}
-
-// BudgetControl returns the model's token-budget control and whether it has one.
-// The returned Control is a deep copy: its Min/Max are independent pointers, so a
-// caller cannot mutate the shared catalog through them. Min/Max are nil when that
-// bound is unspecified (a real 0, e.g. Gemini's min: 0, is a non-nil pointer to 0).
-func (c Capability) BudgetControl() (Control, bool) {
-	for _, ctrl := range c.Controls {
-		if ctrl.Kind == ControlBudget {
-			return ctrl.clone(), true
-		}
-	}
-	return Control{}, false
-}
-
-// HasControl reports whether the model exposes a control of the given kind.
-func (c Capability) HasControl(kind ControlKind) bool {
-	for _, ctrl := range c.Controls {
-		if ctrl.Kind == kind {
-			return true
-		}
-	}
-	return false
-}

internal/reasoning/catalog.go

@@ -1,99 +0,0 @@
-package reasoning
-
-import (
-	_ "embed"
-	"encoding/json"
-	"fmt"
-	"strings"
-)
-
-// modelsdev_snapshot.json is a trimmed snapshot of https://models.dev/api.json:
-// for a set of first-party providers, each model's api id mapped to its
-// reasoning capability (the `reasoning` flag and `reasoning_options`). Routers
-// (OpenRouter/Azure/etc.) are intentionally excluded — they carry generic or
-// stale option lists, so a lookup resolves to the first-party provider instead.
-//
-// Provenance is recorded in the snapshot's `_source`/`_fetched` header fields.
-// To regenerate: fetch api.json, then for each provider in
-// {anthropic, openai, google, xai, groq, deepseek} keep `{reasoning,
-// reasoning_options}` per api id, and bump `_fetched`. Runtime auto-refresh is a
-// separate change.
-//
-//go:embed modelsdev_snapshot.json
-var snapshotBytes []byte
-
-// snapshotDoc is the on-disk shape of the embedded snapshot.
-type snapshotDoc struct {
-	Providers map[string]map[string]Capability `json:"providers"`
-}
-
-// Catalog is a reasoning-capability lookup keyed by provider slug and api model
-// id. It is read-only after construction.
-type Catalog struct {
-	byProvider map[string]map[string]Capability
-}
-
-// ParseCatalog builds a Catalog from a models.dev-style snapshot document.
-func ParseCatalog(data []byte) (Catalog, error) {
-	var doc snapshotDoc
-	if err := json.Unmarshal(data, &doc); err != nil {
-		return Catalog{}, fmt.Errorf("reasoning: parse catalog: %w", err)
-	}
-	if len(doc.Providers) == 0 {
-		return Catalog{}, fmt.Errorf("reasoning: catalog has no providers")
-	}
-	return Catalog{byProvider: doc.Providers}, nil
-}
-
-// embedded is the catalog parsed from the bundled snapshot. A malformed embed is
-// a build/release error, so it panics at init rather than failing silently.
-var embedded = mustParseEmbedded()
-
-func mustParseEmbedded() Catalog {
-	c, err := ParseCatalog(snapshotBytes)
-	if err != nil {
-		panic("reasoning: invalid embedded snapshot: " + err.Error())
-	}
-	return c
-}
-
-// Embedded returns the catalog parsed from the bundled models.dev snapshot.
-func Embedded() Catalog { return embedded }
-
-// Lookup returns the reasoning capability for a model identified by its Zero
-// provider kind and the API model id — the provider's wire name (e.g.
-// "claude-opus-4-1-20250805"), NOT the friendly registry id ("claude-opus-4.1").
-// The match is exact: the api model id is only trimmed, not case-folded, and no
-// router prefixes ("openai/…") or suffixes (":cloud") are stripped. ok is false
-// when the provider or model is not in the snapshot, in which case the caller
-// falls back to its next capability source.
-func (c Catalog) Lookup(provider, apiModel string) (Capability, bool) {
-	apiModel = strings.TrimSpace(apiModel)
-	if apiModel == "" {
-		return Capability{}, false
-	}
-	for _, slug := range providerSlugs(provider) {
-		models, ok := c.byProvider[slug]
-		if !ok {
-			continue
-		}
-		if entry, ok := models[apiModel]; ok {
-			// Deep copy so callers cannot mutate the shared catalog through the
-			// returned Controls / Values slices or budget pointers.
-			return entry.clone(), true
-		}
-	}
-	return Capability{}, false
-}
-
-// providerSlugs maps a Zero provider kind to the models.dev provider slug to
-// look up. Gemini and Google both resolve to "google" (AI Studio), the
-// first-party source for Gemini models.
-func providerSlugs(provider string) []string {
-	switch strings.ToLower(strings.TrimSpace(provider)) {
-	case "gemini":
-		return []string{"google"}
-	default:
-		return []string{strings.ToLower(strings.TrimSpace(provider))}
-	}
-}

internal/reasoning/catalog_test.go

@@ -1,249 +0,0 @@
-package reasoning
-
-import "testing"
-
-func TestEmbeddedCatalogLoads(t *testing.T) {
-	c := Embedded()
-	if len(c.byProvider) == 0 {
-		t.Fatal("embedded catalog is empty")
-	}
-	if _, ok := c.Lookup("openai", "gpt-5"); !ok {
-		t.Fatal("embedded catalog missing a well-known model (gpt-5)")
-	}
-}
-
-func TestParseCatalogRejectsEmpty(t *testing.T) {
-	if _, err := ParseCatalog([]byte(`{}`)); err == nil {
-		t.Fatal("expected an error for a catalog with no providers")
-	}
-	if _, err := ParseCatalog([]byte(`not json`)); err == nil {
-		t.Fatal("expected an error for malformed json")
-	}
-}
-
-func TestGroundTruthOpenAI(t *testing.T) {
-	c := Embedded()
-	cases := []struct {
-		api    string
-		reason bool
-		kind   ControlKind
-		values []string
-	}{
-		{"gpt-5", true, ControlEffort, []string{"minimal", "low", "medium", "high"}},
-		{"gpt-5-codex", true, ControlEffort, []string{"low", "medium", "high"}},
-		{"gpt-5-pro", true, ControlEffort, []string{"high"}},
-		{"gpt-5.1", true, ControlEffort, []string{"none", "low", "medium", "high"}},
-		{"gpt-5.1-codex-max", true, ControlEffort, []string{"low", "medium", "high", "xhigh"}},
-		{"o3", true, ControlEffort, []string{"low", "medium", "high"}},
-	}
-	for _, tc := range cases {
-		entry, ok := c.Lookup("openai", tc.api)
-		if !ok {
-			t.Errorf("%s: not found", tc.api)
-			continue
-		}
-		if entry.Supported() != tc.reason {
-			t.Errorf("%s: Supported=%v want %v", tc.api, entry.Supported(), tc.reason)
-		}
-		if !equalStrings(entry.EffortValues(), tc.values) {
-			t.Errorf("%s: efforts=%v want %v", tc.api, entry.EffortValues(), tc.values)
-		}
-	}
-
-	// Non-reasoning models report reasoning:false and expose no effort.
-	for _, api := range []string{"gpt-4o", "gpt-4o-mini", "gpt-4.1", "gpt-4-turbo"} {
-		entry, ok := c.Lookup("openai", api)
-		if !ok {
-			t.Errorf("%s: not found", api)
-			continue
-		}
-		if entry.Supported() {
-			t.Errorf("%s: Supported=true, want false (non-reasoning)", api)
-		}
-		if entry.EffortValues() != nil {
-			t.Errorf("%s: efforts=%v, want nil", api, entry.EffortValues())
-		}
-	}
-
-	// Version splits: minimal vs none are not interchangeable.
-	gpt5, _ := c.Lookup("openai", "gpt-5")
-	if !gpt5.SupportsEffort("minimal") || gpt5.SupportsEffort("none") {
-		t.Error("gpt-5 should support minimal, not none")
-	}
-	gpt51, _ := c.Lookup("openai", "gpt-5.1")
-	if gpt51.SupportsEffort("minimal") || !gpt51.SupportsEffort("none") {
-		t.Error("gpt-5.1 should support none, not minimal")
-	}
-}
-
-func TestGroundTruthAnthropic(t *testing.T) {
-	c := Embedded()
-
-	// Legacy / budget-only models: a thinking-token budget, no effort enum. The
-	// budget carries an explicit min (1024) and no max — Max stays nil.
-	for _, api := range []string{
-		"claude-opus-4-1-20250805", "claude-sonnet-4-5-20250929", "claude-haiku-4-5-20251001",
-	} {
-		entry, ok := c.Lookup("anthropic", api)
-		if !ok {
-			t.Errorf("%s: not found", api)
-			continue
-		}
-		ctrl, ok := entry.BudgetControl()
-		if !ok {
-			t.Errorf("%s: expected a budget control", api)
-			continue
-		}
-		if ctrl.Min == nil || *ctrl.Min != 1024 {
-			t.Errorf("%s: budget Min=%v, want non-nil 1024", api, ctrl.Min)
-		}
-		if ctrl.Max != nil {
-			t.Errorf("%s: budget Max=%d, want nil (unbounded)", api, *ctrl.Max)
-		}
-		if _, ok := entry.EffortControl(); ok {
-			t.Errorf("%s: did not expect an effort control", api)
-		}
-	}
-
-	// Newer models: native effort enum with xhigh and the budget control removed.
-	opus48, ok := c.Lookup("anthropic", "claude-opus-4-8")
-	if !ok {
-		t.Fatal("claude-opus-4-8 not found")
-	}
-	if !equalStrings(opus48.EffortValues(), []string{"low", "medium", "high", "xhigh", "max"}) {
-		t.Errorf("opus-4-8 efforts=%v", opus48.EffortValues())
-	}
-	if _, ok := opus48.BudgetControl(); ok {
-		t.Error("opus-4-8 should not carry a budget control (removed)")
-	}
-}
-
-func TestGroundTruthGemini(t *testing.T) {
-	c := Embedded()
-
-	pro, ok := c.Lookup("gemini", "gemini-2.5-pro") // gemini kind -> google slug
-	if !ok {
-		t.Fatal("gemini-2.5-pro not found via the gemini provider kind")
-	}
-	ctrl, ok := pro.BudgetControl()
-	if !ok || ctrl.Min == nil || *ctrl.Min != 128 || ctrl.Max == nil || *ctrl.Max != 32768 {
-		t.Errorf("gemini-2.5-pro budget=%+v ok=%v, want min=128 max=32768", ctrl, ok)
-	}
-	if _, ok := pro.EffortControl(); ok {
-		t.Error("gemini-2.5-pro should be budget-only, no effort control")
-	}
-
-	// gemini-2.5-flash can disable thinking: a toggle plus a budget whose explicit
-	// min: 0 must survive (a plain int + omitempty would drop it).
-	flash, _ := c.Lookup("google", "gemini-2.5-flash")
-	if !flash.HasControl(ControlToggle) {
-		t.Error("gemini-2.5-flash should expose a toggle (thinking can be disabled)")
-	}
-	if fctrl, ok := flash.BudgetControl(); !ok || fctrl.Min == nil || *fctrl.Min != 0 {
-		t.Errorf("gemini-2.5-flash budget Min=%v, want a non-nil pointer to 0", fctrl.Min)
-	}
-
-	// Gemini 3.x switched to an effort enum.
-	if g3, ok := c.Lookup("google", "gemini-3-pro-preview"); ok {
-		if _, ok := g3.EffortControl(); !ok {
-			t.Error("gemini-3-pro-preview should expose an effort control")
-		}
-	}
-}
-
-// TestCoversZeroShippedReasoningModels pins that every reasoning model Zero
-// currently ships resolves in the catalog (by its api model id), so the
-// models.dev fallback actually covers Zero's catalog rather than just well-known
-// ids. The api ids mirror internal/modelregistry's curated entries.
-func TestCoversZeroShippedReasoningModels(t *testing.T) {
-	c := Embedded()
-	shipped := []struct {
-		provider, api string
-		wantKind      ControlKind
-	}{
-		{"anthropic", "claude-opus-4-1-20250805", ControlBudget},
-		{"anthropic", "claude-sonnet-4-5-20250929", ControlBudget},
-		{"anthropic", "claude-haiku-4-5-20251001", ControlBudget},
-		{"google", "gemini-2.5-pro", ControlBudget},
-		{"google", "gemini-2.5-flash", ControlToggle},
-		{"google", "gemini-2.5-flash-lite", ControlToggle},
-	}
-	for _, m := range shipped {
-		entry, ok := c.Lookup(m.provider, m.api)
-		if !ok {
-			t.Errorf("%s/%s: not covered by the snapshot", m.provider, m.api)
-			continue
-		}
-		if !entry.Supported() {
-			t.Errorf("%s/%s: Supported=false, want a reasoning model", m.provider, m.api)
-		}
-		if !entry.HasControl(m.wantKind) {
-			t.Errorf("%s/%s: missing expected control %q (controls=%+v)", m.provider, m.api, m.wantKind, entry.Controls)
-		}
-	}
-}
-
-// TestLookupReturnsDeepCopy pins that a caller cannot corrupt the shared
-// embedded catalog by mutating a returned Capability's slices.
-func TestLookupReturnsDeepCopy(t *testing.T) {
-	c := Embedded()
-	first, ok := c.Lookup("openai", "gpt-5")
-	if !ok || len(first.Controls) == 0 || len(first.Controls[0].Values) == 0 {
-		t.Fatal("setup: gpt-5 should have an effort control with values")
-	}
-	first.Controls[0].Values[0] = "MUTATED"
-	first.Controls[0].Kind = "MUTATED"
-
-	second, _ := c.Lookup("openai", "gpt-5")
-	if second.Controls[0].Values[0] == "MUTATED" || second.Controls[0].Kind == "MUTATED" {
-		t.Error("Lookup leaked a shared reference; a caller's mutation reached the catalog")
-	}
-}
-
-// TestAccessorsReturnDeepCopies pins that EffortControl/BudgetControl hand back
-// independent copies, so a caller holding a Capability that shares state with the
-// catalog cannot corrupt it by mutating an accessor result's slice or pointers.
-func TestAccessorsReturnDeepCopies(t *testing.T) {
-	min := 100
-	c := Capability{Reasoning: true, Controls: []Control{
-		{Kind: ControlEffort, Values: []string{"low", "high"}},
-		{Kind: ControlBudget, Min: &min},
-	}}
-
-	eff, _ := c.EffortControl()
-	eff.Values[0] = "MUTATED"
-	if c.Controls[0].Values[0] != "low" {
-		t.Error("EffortControl leaked the Values slice")
-	}
-
-	bud, _ := c.BudgetControl()
-	*bud.Min = 999
-	if *c.Controls[1].Min != 100 {
-		t.Error("BudgetControl leaked the Min pointer")
-	}
-}
-
-func TestLookupMissFallsThrough(t *testing.T) {
-	c := Embedded()
-	if _, ok := c.Lookup("openai", "totally-made-up-model"); ok {
-		t.Error("unknown model should miss")
-	}
-	if _, ok := c.Lookup("no-such-provider", "gpt-5"); ok {
-		t.Error("unknown provider should miss")
-	}
-	if _, ok := c.Lookup("openai", "  "); ok {
-		t.Error("blank api model should miss")
-	}
-}
-
-func equalStrings(a, b []string) bool {
-	if len(a) != len(b) {
-		return false
-	}
-	for i := range a {
-		if a[i] != b[i] {
-			return false
-		}
-	}
-	return true
-}

internal/reltime/reltime.go

@@ -1,65 +1 @@
 package reltime
-
-import "fmt"
-
-// RelTime renders a duration in seconds as a human-readable relative-time
-// string. This is the ORIGINAL implementation, kept verbatim so
-// characterization tests can be pinned against it before refactoring.
-func RelTime(d int) string {
-	if d < 0 {
-		d = -d
-		if d < 60 {
-			if d == 1 {
-				return "1 second ago"
-			}
-			return fmt.Sprintf("%d seconds ago", d)
-		}
-		if d < 3600 {
-			m := d / 60
-			if m == 1 {
-				return "1 minute ago"
-			}
-			return fmt.Sprintf("%d minutes ago", m)
-		}
-		if d < 86400 {
-			h := d / 3600
-			if h == 1 {
-				return "1 hour ago"
-			}
-			return fmt.Sprintf("%d hours ago", h)
-		}
-		x := d / 86400
-		if x == 1 {
-			return "1 day ago"
-		}
-		return fmt.Sprintf("%d days ago", x)
-	} else if d == 0 {
-		return "just now"
-	} else {
-		if d < 60 {
-			if d == 1 {
-				return "in 1 second"
-			}
-			return fmt.Sprintf("in %d seconds", d)
-		}
-		if d < 3600 {
-			m := d / 60
-			if m == 1 {
-				return "in 1 minute"
-			}
-			return fmt.Sprintf("in %d minutes", m)
-		}
-		if d < 86400 {
-			h := d / 3600
-			if h == 1 {
-				return "in 1 hour"
-			}
-			return fmt.Sprintf("in %d hours", h)
-		}
-		x := d / 86400
-		if x == 1 {
-			return "in 1 day"
-		}
-		return fmt.Sprintf("in %d days", x)
-	}
-}

internal/repomap/export_test.go

@@ -0,0 +1,12 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package repomap
+
+import (
+	"io/fs"
+
+	"github.com/Gitlawb/zero/internal/workspaceindex"
+)
+
+func handleWalkError(cleanRoot string, current string, entry fs.DirEntry, walkErr error, truncated *bool) (bool, error) {
+	return workspaceindex.HandleWalkError(cleanRoot, current, entry, walkErr, truncated)
+}

internal/repomap/prompt.go

@@ -188,7 +188,7 @@ func writePromptLine(builder *strings.Builder, format string, args ...any) {
 	if builder.Len() > 0 {
 		builder.WriteByte('\n')
 	}
-	builder.WriteString(fmt.Sprintf(format, args...))
+	fmt.Fprintf(builder, format, args...)
 }
 
 func clampBudget(text string, budget int) string {

internal/repomap/repomap.go

@@ -2,7 +2,6 @@
 package repomap
 
 import (
-	"io/fs"
 	"sort"
 	"strings"
 
@@ -82,10 +81,6 @@ func Scan(root string, options Options) (Snapshot, error) {
 	return snapshot, err
 }
 
-func handleWalkError(cleanRoot string, current string, entry fs.DirEntry, walkErr error, truncated *bool) (bool, error) {
-	return workspaceindex.HandleWalkError(cleanRoot, current, entry, walkErr, truncated)
-}
-
 func fileDepth(rel string) int {
 	return workspaceindex.FileDepth(rel)
 }

internal/review/export_test.go

@@ -0,0 +1,11 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package review
+
+func HasBlockingChecks(checks []Check) bool {
+	for _, check := range checks {
+		if IsBlocking(check.Outcome) {
+			return true
+		}
+	}
+	return false
+}

internal/review/review.go

@@ -76,15 +76,6 @@ func BuildChecksFromEnv(env map[string]string) []Check {
 	return checks
 }
 
-func HasBlockingChecks(checks []Check) bool {
-	for _, check := range checks {
-		if IsBlocking(check.Outcome) {
-			return true
-		}
-	}
-	return false
-}
-
 func IsBlocking(outcome Outcome) bool {
 	switch outcome {
 	case OutcomeFailure, OutcomeCancelled, OutcomeTimedOut, OutcomeActionRequired, OutcomeUnknown:

internal/sandbox/adapters.go

@@ -225,7 +225,7 @@ func (backend Backend) SandboxEnvMarkers(policy Policy) []string {
 	if policy.Mode == ModeDisabled {
 		return nil
 	}
-	if !(backend.CommandWrapping && backend.Available) && backend.Name != BackendWSL {
+	if (!backend.CommandWrapping || !backend.Available) && backend.Name != BackendWSL {
 		return nil
 	}
 	name := backend.Name

internal/sandbox/command_prefix.go

@@ -153,10 +153,7 @@ func unsafeCommandPrefix(prefix []string) bool {
 			return true
 		}
 	}
-	if unsafeCommandPrefixLauncher(prefix[0]) {
-		return true
-	}
-	return false
+	return unsafeCommandPrefixLauncher(prefix[0])
 }
 
 func unsafeCommandPrefixPart(part string) bool {

internal/sandbox/engine.go

@@ -249,7 +249,7 @@ func (engine *Engine) shellSandboxActive(policy Policy) bool {
 		return false
 	}
 	backend := engine.backend
-	if !(backend.Available && backend.Executable != "" && backend.CommandWrapping && backend.NativeIsolation) {
+	if !backend.Available || backend.Executable == "" || !backend.CommandWrapping || !backend.NativeIsolation {
 		return false
 	}
 	// On Windows the command is only actually wrapped once `zero sandbox setup`

internal/sandbox/engine_test.go

@@ -2,6 +2,7 @@ package sandbox
 
 import (
 	"context"
+	"fmt"
 	"os"
 	"path/filepath"
 	"strings"
@@ -485,6 +486,28 @@ func TestEngineDeniesOutOfWorkspacePaths(t *testing.T) {
 	}
 }
 
+func TestEnginePromptsForOutOfWorkspaceReadWithReadSpecificReason(t *testing.T) {
+	root := t.TempDir()
+	outside := outsideDefaultTempPath(root, "external")
+	engine := NewEngine(EngineOptions{WorkspaceRoot: root, Policy: DefaultPolicy()})
+
+	decision := engine.Evaluate(context.Background(), Request{
+		ToolName:      "list_directory",
+		SideEffect:    SideEffectRead,
+		Permission:    PermissionAllow,
+		WorkspaceRoot: root,
+		Args:          map[string]any{"path": outside},
+	})
+
+	want := fmt.Sprintf("Reading %s requires access outside the workspace.", outside)
+	if decision.Action != ActionPrompt || decision.Block == nil || !decision.Block.Recoverable {
+		t.Fatalf("outside read decision = %#v, want recoverable prompt", decision)
+	}
+	if decision.Reason != want || decision.Block.Reason != want {
+		t.Fatalf("outside read reasons = decision %q, block %q; want %q", decision.Reason, decision.Block.Reason, want)
+	}
+}
+
 func TestEnginePrecheckReportsBlocksBeforeExecution(t *testing.T) {
 	root := t.TempDir()
 	outside := outsideDefaultTempPath(root, "escape.txt")

internal/sandbox/export_test.go

@@ -0,0 +1,78 @@
+// Test seams: helpers only test code uses, kept out of the production binary.
+package sandbox
+
+import (
+	"path/filepath"
+	"strings"
+)
+
+func FormatGrantList(grants []Grant) string {
+	return FormatGrantListWithCommandPrefixes(grants, nil)
+}
+
+func DefaultPermissionProfile(workspaceRoot string) PermissionProfile {
+	return PermissionProfileFromPolicy(workspaceRoot, DefaultPolicy(), nil)
+}
+
+// writeRoots returns the full ordered write-root list for command plans:
+// the workspace root plus any granted extra roots. The single-root fallback
+// only applies to engines built without a workspace root (NewEngine always
+// builds a scope otherwise); it is kept as defense in depth.
+func (engine *Engine) writeRoots(workspaceRoot string) []string {
+	var roots []string
+	if engine.scope != nil {
+		roots = engine.scope.Roots()
+	} else {
+		roots = []string{workspaceRoot}
+	}
+	// Reflect the policy's AllowWrite roots in the OS backend write binds so a
+	// sandboxed shell command may write where the policy grants writes. DenyWrite
+	// is enforced at the policy gate, and on sandbox-exec additionally as an
+	// explicit deny rule (see sandboxExecProfile).
+	policy := engine.effectivePolicy(engine.policy)
+	if extra := resolveWriteRootPaths(policy.AllowWrite); len(extra) > 0 {
+		roots = dedupeStrings(append(roots, extra...))
+	}
+	return roots
+}
+
+func sandboxEnvironment(policy Policy, backend BackendName, workspaceRoot string) []string {
+	return sandboxEnvironmentForCommand(nil, policy, backend, workspaceRoot)
+}
+
+func sandboxExecProfile(writeRoots []string, policy Policy, denialTag string) string {
+	return seatbeltProfileFromPermissionProfile(seatbeltCompatibilityPermissionProfile(writeRoots, policy), policy, denialTag)
+}
+
+func seatbeltCompatibilityPermissionProfile(writeRoots []string, policy Policy) PermissionProfile {
+	fs := FileSystemPolicy{
+		Kind:                 FileSystemUnrestricted,
+		ReadRoots:            []string{string(filepath.Separator)},
+		IncludePlatformRoots: true,
+		AllowTemp:            true,
+	}
+	if policy.EnforceWorkspace {
+		fs.Kind = FileSystemRestricted
+		fs.WriteRoots = make([]WritableRoot, 0, len(writeRoots))
+		for _, root := range writeRoots {
+			fs.WriteRoots = append(fs.WriteRoots, WritableRoot{Root: root})
+		}
+	}
+	fs.DenyRead = dedupeStrings(append(normalizeProfilePaths(policy.DenyRead), credentialDenyReadPaths(policy)...))
+	fs.DenyWrite = normalizeProfilePaths(policy.DenyWrite)
+	return PermissionProfile{
+		FileSystem: fs,
+		Network:    NetworkPolicy{Mode: policy.Network},
+	}
+}
+
+// WindowsSandboxSetupPathForRunner derives the setup helper's path from a
+// standalone command-runner path (the sibling .exe in the release layout).
+// Retained for that layout; self-dispatch callers use
+// ResolveWindowsSandboxSetupHelper instead.
+func WindowsSandboxSetupPathForRunner(runnerPath string) string {
+	if strings.TrimSpace(runnerPath) == "" {
+		return ""
+	}
+	return filepath.Join(filepath.Dir(runnerPath), WindowsSandboxSetupName)
+}

internal/sandbox/grants.go

@@ -16,7 +16,11 @@ import (
 	"github.com/Gitlawb/zero/internal/redaction"
 )
 
-const grantSchemaVersion = 3
+const (
+	grantSchemaVersion = 3
+	grantLockTimeout   = 5 * time.Second
+	grantLockRetry     = 10 * time.Millisecond
+)
 
 type Grant struct {
 	ToolName   string        `json:"toolName"`
@@ -139,7 +143,40 @@ func (store *GrantStore) ConsumeMigrationNotice() (string, error) {
 	}
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	// Look for a pending notice WITHOUT taking the interprocess lock. Acquiring
+	// that lock creates the grants directory and opens <grants>.lockfile, so doing
+	// it unconditionally would make this call require a WRITABLE grants directory
+	// even when it has nothing to write. Every startup and every `zero exec` calls
+	// this, so a user with no grants file whose configured grants path (or its
+	// parent) sits on a read-only mount would fail outright with "failed to
+	// migrate sandbox grants", where reading alone reports an empty state and
+	// writes nothing.
+	state, needsMigration, err := store.readStateFile(false)
+	if err != nil {
+		return "", err
+	}
+	if !needsMigration && (state.Migration == nil || !state.Migration.NoticePending) {
+		return "", nil
+	}
+	// Past here a write is genuinely required — either a schema/policy migration,
+	// which only a lock holder may perform, or clearing NoticePending — so the
+	// lock is warranted and a read-only grants path legitimately fails (as it did
+	// before this lock existed, via readState's own migration path).
+	//
+	// Clearing the pending flag is a read-modify-write like any other mutator, so
+	// it takes the same interprocess lock: two frontends starting at once must not
+	// both read NoticePending, both print the notice, and clobber each other's
+	// write of the rest of the state.
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return "", err
+	}
+	defer unlock()
+	// Re-read under the lock. The unlocked peek above is only a snapshot: another
+	// process may have consumed the notice or migrated the file while this one
+	// waited, and when needsMigration was set that snapshot's contents are
+	// meaningless by readStateFile's contract.
+	state, err = store.readStateLocked()
 	if err != nil {
 		return "", err
 	}
@@ -161,7 +198,13 @@ func (store *GrantStore) Grant(input GrantInput) (Grant, error) {
 	}
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return Grant{}, err
+	}
+	defer unlock()
+
+	state, err := store.readStateLocked()
 	if err != nil {
 		return Grant{}, err
 	}
@@ -193,7 +236,13 @@ func (store *GrantStore) GrantCommandPrefix(input CommandPrefixInput) (CommandPr
 	}
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return CommandPrefixGrant{}, err
+	}
+	defer unlock()
+
+	state, err := store.readStateLocked()
 	if err != nil {
 		return CommandPrefixGrant{}, err
 	}
@@ -337,7 +386,13 @@ func (store *GrantStore) Revoke(toolName string) (int, error) {
 	}
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return 0, err
+	}
+	defer unlock()
+
+	state, err := store.readStateLocked()
 	if err != nil {
 		return 0, err
 	}
@@ -372,7 +427,13 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err
 	}
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return 0, err
+	}
+	defer unlock()
+
+	state, err := store.readStateLocked()
 	if err != nil {
 		return 0, err
 	}
@@ -407,7 +468,13 @@ func (store *GrantStore) RevokePath(toolName string, scopePath string) (int, err
 func (store *GrantStore) Clear() (int, error) {
 	store.mu.Lock()
 	defer store.mu.Unlock()
-	state, err := store.readState()
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return 0, err
+	}
+	defer unlock()
+
+	state, err := store.readStateLocked()
 	if err != nil {
 		return 0, err
 	}
@@ -427,10 +494,6 @@ func (store *GrantStore) Clear() (int, error) {
 	return count, nil
 }
 
-func FormatGrantList(grants []Grant) string {
-	return FormatGrantListWithCommandPrefixes(grants, nil)
-}
-
 func FormatGrantListWithCommandPrefixes(grants []Grant, prefixes []CommandPrefixGrant) string {
 	if len(grants) == 0 && len(prefixes) == 0 {
 		return "No persistent sandbox grants."
@@ -541,7 +604,52 @@ func reconcileScope(scope string, kind ScopeKind) (string, ScopeKind) {
 	return scope, kind
 }
 
+// readState reads the grant file for a caller that does NOT hold the
+// interprocess lock (the lookup/list paths). A schema or policy migration has to
+// write — a backup plus the migrated state — so it is performed under the lock,
+// and the read is retaken there because another process may have migrated the
+// file while this one waited.
 func (store *GrantStore) readState() (grantFile, error) {
+	state, needsMigration, err := store.readStateFile(false)
+	if err != nil || !needsMigration {
+		return state, err
+	}
+	unlock, err := store.lockStateFile()
+	if err != nil {
+		return grantFile{}, err
+	}
+	defer unlock()
+	return store.readStateLocked()
+}
+
+// readStateLocked reads the grant file for a caller that already holds the
+// interprocess lock, so a migration writes through directly instead of
+// re-acquiring it (which would deadlock against the caller's own lock).
+func (store *GrantStore) readStateLocked() (grantFile, error) {
+	state, _, err := store.readStateFile(true)
+	return state, err
+}
+
+// readStateFile decodes the grant file. needsMigration reports that the state on
+// disk must be rewritten (legacy schema or a changed policy version) and that the
+// caller has to retake this read while holding the interprocess lock; the
+// returned state is meaningless in that case.
+func (store *GrantStore) readStateFile(locked bool) (grantFile, bool, error) {
+	state, err := store.decodeState(locked)
+	if err != nil {
+		if errors.Is(err, errGrantMigrationNeedsLock) {
+			return grantFile{}, true, nil
+		}
+		return grantFile{}, false, err
+	}
+	return state, false, nil
+}
+
+// errGrantMigrationNeedsLock is the internal signal from decodeState that the
+// file needs a migration write, which only a lock holder may perform.
+var errGrantMigrationNeedsLock = errors.New("sandbox grants file needs a migration write")
+
+func (store *GrantStore) decodeState(locked bool) (grantFile, error) {
 	data, err := os.ReadFile(store.filePath)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
@@ -563,12 +671,18 @@ func (store *GrantStore) readState() (grantFile, error) {
 		return grantFile{}, store.invalidGrantFile(err)
 	}
 	if head.SchemaVersion == 1 || head.SchemaVersion == 2 {
+		if !locked {
+			return grantFile{}, errGrantMigrationNeedsLock
+		}
 		return store.migrateLegacyState(data, head.SchemaVersion, head.Grants, head.CommandPrefixes)
 	}
 	if head.SchemaVersion != grantSchemaVersion {
 		return grantFile{}, fmt.Errorf("invalid sandbox grants file at %s: unsupported schemaVersion", store.filePath)
 	}
 	if head.PolicyVersion != execution.PolicyVersion {
+		if !locked {
+			return grantFile{}, errGrantMigrationNeedsLock
+		}
 		return store.migrateChangedPolicy(data, head.PolicyVersion, head.Grants, head.CommandPrefixes)
 	}
 	buckets := map[string][]Grant{}
@@ -804,6 +918,36 @@ func (store *GrantStore) writeState(state grantFile) error {
 	return nil
 }
 
+func (store *GrantStore) lockStateFile() (func(), error) {
+	lockPath := store.filePath + ".lockfile"
+	if err := os.MkdirAll(filepath.Dir(store.filePath), 0o700); err != nil {
+		return nil, err
+	}
+	file, err := os.OpenFile(lockPath, os.O_CREATE|os.O_RDWR, 0o600)
+	if err != nil {
+		return nil, err
+	}
+	deadline := time.Now().Add(grantLockTimeout)
+	for {
+		locked, err := tryLockGrantFile(file)
+		if err != nil {
+			_ = file.Close()
+			return nil, err
+		}
+		if locked {
+			return func() {
+				_ = unlockGrantFile(file)
+				_ = file.Close()
+			}, nil
+		}
+		if time.Now().After(deadline) {
+			_ = file.Close()
+			return nil, fmt.Errorf("timed out waiting for sandbox grants lock at %s", lockPath)
+		}
+		time.Sleep(grantLockRetry)
+	}
+}
+
 func normalizeStoredGrant(name string, grant Grant) (Grant, error) {
 	if strings.TrimSpace(grant.ToolName) == "" {
 		grant.ToolName = name

internal/sandbox/grants_lock_unix.go

@@ -0,0 +1,24 @@
+//go:build !windows
+
+package sandbox
+
+import (
+	"errors"
+	"os"
+	"syscall"
+)
+
+func tryLockGrantFile(file *os.File) (bool, error) {
+	err := syscall.Flock(int(file.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
+	if err == nil {
+		return true, nil
+	}
+	if errors.Is(err, syscall.EWOULDBLOCK) || errors.Is(err, syscall.EAGAIN) {
+		return false, nil
+	}
+	return false, err
+}
+
+func unlockGrantFile(file *os.File) error {
+	return syscall.Flock(int(file.Fd()), syscall.LOCK_UN)
+}

internal/sandbox/grants_lock_windows.go

@@ -0,0 +1,60 @@
+//go:build windows
+
+package sandbox
+
+import (
+	"errors"
+	"os"
+	"syscall"
+	"unsafe"
+)
+
+const (
+	grantLockfileFailImmediately = 0x00000001
+	grantLockfileExclusiveLock   = 0x00000002
+	grantErrorLockViolation      = syscall.Errno(33)
+	grantErrorSharingViolation   = syscall.Errno(32)
+)
+
+var (
+	grantKernel32         = syscall.NewLazyDLL("kernel32.dll")
+	grantProcLockFileEx   = grantKernel32.NewProc("LockFileEx")
+	grantProcUnlockFileEx = grantKernel32.NewProc("UnlockFileEx")
+)
+
+func tryLockGrantFile(file *os.File) (bool, error) {
+	var overlapped syscall.Overlapped
+	result, _, err := grantProcLockFileEx.Call(
+		file.Fd(),
+		uintptr(grantLockfileExclusiveLock|grantLockfileFailImmediately),
+		0,
+		1,
+		0,
+		uintptr(unsafe.Pointer(&overlapped)),
+	)
+	if result != 0 {
+		return true, nil
+	}
+	if errors.Is(err, grantErrorLockViolation) || errors.Is(err, grantErrorSharingViolation) {
+		return false, nil
+	}
+	if err == syscall.Errno(0) {
+		return false, nil
+	}
+	return false, err
+}
+
+func unlockGrantFile(file *os.File) error {
+	var overlapped syscall.Overlapped
+	result, _, err := grantProcUnlockFileEx.Call(
+		file.Fd(),
+		0,
+		1,
+		0,
+		uintptr(unsafe.Pointer(&overlapped)),
+	)
+	if result != 0 || err == syscall.Errno(0) {
+		return nil
+	}
+	return err
+}

internal/sandbox/grants_test.go

@@ -1,10 +1,13 @@
 package sandbox
 
 import (
+	"errors"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
+	"time"
 )
 
 func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) {
@@ -70,6 +73,246 @@ func TestGrantStorePersistsListsRevokesAndClears(t *testing.T) {
 	}
 }
 
+func TestGrantStoreSerializesWritesAcrossStores(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "sandbox-grants.json")
+	storeA, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore A returned error: %v", err)
+	}
+	storeB, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore B returned error: %v", err)
+	}
+
+	unlock, err := storeA.lockStateFile()
+	if err != nil {
+		t.Fatalf("lockStateFile returned error: %v", err)
+	}
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := storeB.Grant(GrantInput{ToolName: "write_file", Decision: GrantAllow})
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		unlock()
+		t.Fatalf("Grant completed while another store held the file lock: %v", err)
+	case <-time.After(100 * time.Millisecond):
+		// Expected: storeB is waiting for storeA to release the lock.
+	}
+
+	unlock()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("Grant returned error after file lock release: %v", err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("Grant did not complete after file lock release")
+	}
+}
+
+// TestGrantStoreSerializesMigrationNoticeAcrossStores covers the other
+// read-modify-write on the state file: clearing the pending flag. Two frontends
+// starting at once must not both read it as pending and clobber each other.
+func TestGrantStoreSerializesMigrationNoticeAcrossStores(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "sandbox-grants.json")
+	if err := writeText(path, `{"schemaVersion":1,"grants":{"write_file":{"toolName":"write_file","decision":"allow","approvedAt":"2026-06-05T14:30:00Z"}}}`); err != nil {
+		t.Fatalf("write v1 grants: %v", err)
+	}
+	storeA, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore A returned error: %v", err)
+	}
+	storeB, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore B returned error: %v", err)
+	}
+	// Migrate first, so the notice is pending and the lock contention below is
+	// about consuming it rather than about the migration write.
+	if _, err := storeA.List(); err != nil {
+		t.Fatalf("List returned error: %v", err)
+	}
+
+	unlock, err := storeA.lockStateFile()
+	if err != nil {
+		t.Fatalf("lockStateFile returned error: %v", err)
+	}
+	done := make(chan error, 1)
+	go func() {
+		notice, err := storeB.ConsumeMigrationNotice()
+		if err == nil && notice == "" {
+			err = errors.New("expected the pending migration notice")
+		}
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		unlock()
+		t.Fatalf("ConsumeMigrationNotice completed while another store held the file lock: %v", err)
+	case <-time.After(100 * time.Millisecond):
+		// Expected: storeB is waiting for storeA to release the lock.
+	}
+
+	unlock()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("ConsumeMigrationNotice after lock release: %v", err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("ConsumeMigrationNotice did not complete after file lock release")
+	}
+	// The flag write landed, so the notice is not reported twice.
+	if again, err := storeA.ConsumeMigrationNotice(); err != nil || again != "" {
+		t.Fatalf("second ConsumeMigrationNotice = %q err=%v, want empty", again, err)
+	}
+}
+
+// TestConsumeMigrationNoticeWritesNothingWithoutAPendingNotice is the regression
+// test for jatmn's #755 finding: ConsumeMigrationNotice took the interprocess
+// lock unconditionally, and acquiring that lock MkdirAlls the grants directory
+// and creates <grants>.lockfile. Every startup and every `zero exec` calls this,
+// so it turned "read the grants state" into "require a writable grants
+// directory" — a user with no grants file whose grants path sat on a read-only
+// mount failed outright with "failed to migrate sandbox grants".
+//
+// Asserting that nothing is created is the portable form of that requirement: it
+// holds identically on every OS and as any user, unlike a read-only-directory
+// test (see the sibling test, which cannot run everywhere).
+func TestConsumeMigrationNoticeWritesNothingWithoutAPendingNotice(t *testing.T) {
+	dir := filepath.Join(t.TempDir(), "config")
+	path := filepath.Join(dir, "sandbox-grants.json")
+	store, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore returned error: %v", err)
+	}
+
+	notice, err := store.ConsumeMigrationNotice()
+	if err != nil {
+		t.Fatalf("ConsumeMigrationNotice with no grants file: %v", err)
+	}
+	if notice != "" {
+		t.Fatalf("notice = %q, want empty when nothing was migrated", notice)
+	}
+	// Nothing at all may be created: not the lock file, not the grants file, not
+	// even the directory that would hold them.
+	if _, err := os.Lstat(dir); !os.IsNotExist(err) {
+		entries, _ := os.ReadDir(dir)
+		names := make([]string, 0, len(entries))
+		for _, entry := range entries {
+			names = append(names, entry.Name())
+		}
+		t.Fatalf("the grants directory was created (contents: %v); reading a state with no pending notice must not require a writable directory", names)
+	}
+}
+
+// TestConsumeMigrationNoticeToleratesAReadOnlyGrantsDirectory exercises the
+// actual failure from the same finding, rather than only its portable proxy
+// above: with the grants directory read-only and no grants file, the pre-fix
+// code failed at MkdirAll/OpenFile for the lock file.
+//
+// It cannot run everywhere — Windows ignores the mode bits used here, and root
+// bypasses directory permissions entirely — so it skips instead of pretending to
+// cover those platforms.
+func TestConsumeMigrationNoticeToleratesAReadOnlyGrantsDirectory(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("Unix directory mode bits do not deny writes on Windows")
+	}
+	if os.Geteuid() == 0 {
+		t.Skip("root bypasses directory permissions, so a read-only directory cannot be simulated")
+	}
+	dir := t.TempDir()
+	path := filepath.Join(dir, "sandbox-grants.json")
+	store, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore returned error: %v", err)
+	}
+	if err := os.Chmod(dir, 0o500); err != nil {
+		t.Skipf("cannot make the grants directory read-only: %v", err)
+	}
+	// Restore write permission so t.TempDir's cleanup can remove the directory.
+	t.Cleanup(func() { _ = os.Chmod(dir, 0o700) })
+	// Confirm the directory really is read-only here; otherwise this test would
+	// pass without exercising anything.
+	if probe, err := os.OpenFile(filepath.Join(dir, ".probe"), os.O_CREATE|os.O_RDWR, 0o600); err == nil {
+		_ = probe.Close()
+		t.Skip("this filesystem still allows writes to a 0500 directory")
+	}
+
+	notice, err := store.ConsumeMigrationNotice()
+	if err != nil {
+		t.Fatalf("ConsumeMigrationNotice on a read-only grants directory: %v", err)
+	}
+	if notice != "" {
+		t.Fatalf("notice = %q, want empty when nothing was migrated", notice)
+	}
+}
+
+// TestGrantStoreMigrationTakesTheFileLock covers the migrations that write
+// through readState: a legacy file must not be rewritten (nor its backup written)
+// while another process holds the lock, or two processes could both migrate and
+// one would clobber the other's result.
+func TestGrantStoreMigrationTakesTheFileLock(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "sandbox-grants.json")
+	original := `{"schemaVersion":1,"grants":{"write_file":{"toolName":"write_file","decision":"allow","approvedAt":"2026-06-05T14:30:00Z"}}}`
+	if err := writeText(path, original); err != nil {
+		t.Fatalf("write v1 grants: %v", err)
+	}
+	storeA, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore A returned error: %v", err)
+	}
+	storeB, err := NewGrantStore(StoreOptions{FilePath: path})
+	if err != nil {
+		t.Fatalf("NewGrantStore B returned error: %v", err)
+	}
+
+	unlock, err := storeA.lockStateFile()
+	if err != nil {
+		t.Fatalf("lockStateFile returned error: %v", err)
+	}
+	done := make(chan error, 1)
+	go func() {
+		_, err := storeB.List()
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		unlock()
+		t.Fatalf("the migration ran while another store held the file lock: %v", err)
+	case <-time.After(100 * time.Millisecond):
+		if raw, err := os.ReadFile(path); err != nil {
+			unlock()
+			t.Fatalf("ReadFile grants: %v", err)
+		} else if string(raw) != original {
+			unlock()
+			t.Fatalf("the grant file was rewritten while the lock was held:\n%s", raw)
+		}
+	}
+
+	unlock()
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("List after lock release: %v", err)
+		}
+	case <-time.After(2 * time.Second):
+		t.Fatal("the migration did not complete after file lock release")
+	}
+	raw, err := os.ReadFile(path)
+	if err != nil {
+		t.Fatalf("ReadFile migrated grants: %v", err)
+	}
+	if !strings.Contains(string(raw), `"schemaVersion": 3`) {
+		t.Fatalf("grant file was not migrated after the lock was released:\n%s", raw)
+	}
+}
+
 func TestGrantStoreMigratesExactV1GrantAndReportsOnce(t *testing.T) {
 	root := t.TempDir()
 	path := filepath.Join(root, "sandbox-grants.json")

internal/sandbox/landlock_linux.go

@@ -38,7 +38,7 @@ func ApplyLandlockFilesystemProfile(profile PermissionProfile, cwd string) error
 	}
 	writeRoots := landlockWritableRoots(profile.FileSystem, cwd)
 	if len(writeRoots) == 0 {
-		return errors.New("Landlock requires at least one writable root")
+		return errors.New("Landlock requires at least one writable root") //nolint:staticcheck // Preserve the proper name and established user-facing text.
 	}
 	return installLandlockFilesystemRules(writeRoots)
 }

internal/sandbox/landlock_other.go

@@ -4,7 +4,7 @@ package sandbox
 
 import "errors"
 
-var ErrLandlockUnsupported = errors.New("Landlock is only supported on Linux")
+var ErrLandlockUnsupported = errors.New("Landlock is only supported on Linux") //nolint:staticcheck // Preserve the exported sentinel's established text.
 
 func ApplyLandlockFilesystemProfile(profile PermissionProfile, cwd string) error {
 	return ErrLandlockUnsupported

internal/sandbox/profile.go

@@ -69,10 +69,6 @@ func gitMetadataWriteCarveouts(root string) []string {
 	}
 }
 
-func DefaultPermissionProfile(workspaceRoot string) PermissionProfile {
-	return PermissionProfileFromPolicy(workspaceRoot, DefaultPolicy(), nil)
-}
-
 func PermissionProfileFromPolicy(workspaceRoot string, policy Policy, scope *Scope) PermissionProfile {
 	if policy.Mode == "" {
 		policy = DefaultPolicy()

internal/sandbox/runner.go

@@ -143,28 +143,6 @@ func (engine *Engine) PrepareExecution(ctx context.Context, request execution.Re
 	}, nil
 }
 
-// writeRoots returns the full ordered write-root list for command plans:
-// the workspace root plus any granted extra roots. The single-root fallback
-// only applies to engines built without a workspace root (NewEngine always
-// builds a scope otherwise); it is kept as defense in depth.
-func (engine *Engine) writeRoots(workspaceRoot string) []string {
-	var roots []string
-	if engine.scope != nil {
-		roots = engine.scope.Roots()
-	} else {
-		roots = []string{workspaceRoot}
-	}
-	// Reflect the policy's AllowWrite roots in the OS backend write binds so a
-	// sandboxed shell command may write where the policy grants writes. DenyWrite
-	// is enforced at the policy gate, and on sandbox-exec additionally as an
-	// explicit deny rule (see sandboxExecProfile).
-	policy := engine.effectivePolicy(engine.policy)
-	if extra := resolveWriteRootPaths(policy.AllowWrite); len(extra) > 0 {
-		roots = dedupeStrings(append(roots, extra...))
-	}
-	return roots
-}
-
 func (engine *Engine) BuildCommandPlan(spec CommandSpec) (CommandPlan, error) {
 	if engine == nil {
 		return directCommandPlan(spec, Backend{Name: BackendUnavailable, Message: "sandbox disabled"}, Policy{}, ""), nil
@@ -453,32 +431,6 @@ func seatbeltCommandPlanWithProfile(spec CommandSpec, workspaceRoot string, prof
 	return plan
 }
 
-func seatbeltCompatibilityPermissionProfile(writeRoots []string, policy Policy) PermissionProfile {
-	fs := FileSystemPolicy{
-		Kind:                 FileSystemUnrestricted,
-		ReadRoots:            []string{string(filepath.Separator)},
-		IncludePlatformRoots: true,
-		AllowTemp:            true,
-	}
-	if policy.EnforceWorkspace {
-		fs.Kind = FileSystemRestricted
-		fs.WriteRoots = make([]WritableRoot, 0, len(writeRoots))
-		for _, root := range writeRoots {
-			fs.WriteRoots = append(fs.WriteRoots, WritableRoot{Root: root})
-		}
-	}
-	fs.DenyRead = dedupeStrings(append(normalizeProfilePaths(policy.DenyRead), credentialDenyReadPaths(policy)...))
-	fs.DenyWrite = normalizeProfilePaths(policy.DenyWrite)
-	return PermissionProfile{
-		FileSystem: fs,
-		Network:    NetworkPolicy{Mode: policy.Network},
-	}
-}
-
-func sandboxEnvironment(policy Policy, backend BackendName, workspaceRoot string) []string {
-	return sandboxEnvironmentForCommand(nil, policy, backend, workspaceRoot)
-}
-
 func sandboxEnvironmentForCommand(specEnv []string, policy Policy, backend BackendName, workspaceRoot string) []string {
 	return sandboxEnvironmentForCommandWithSensitiveEnv(specEnv, policy, backend, workspaceRoot, nil)
 }
@@ -664,10 +616,6 @@ func sandboxMachLookupRule() string {
 	return "(allow mach-lookup\n  " + strings.Join(filters, "\n  ") + ")"
 }
 
-func sandboxExecProfile(writeRoots []string, policy Policy, denialTag string) string {
-	return seatbeltProfileFromPermissionProfile(seatbeltCompatibilityPermissionProfile(writeRoots, policy), policy, denialTag)
-}
-
 func seatbeltProfileFromPermissionProfile(profile PermissionProfile, policy Policy, denialTag string) string {
 	networkRule := networkRuleForProfile(profile.Network)
 	readRule := seatbeltReadRule(profile.FileSystem)

internal/sandbox/scope.go

@@ -203,11 +203,24 @@ func dedupeScopeRoots(roots []string) []string {
 // only on outside_workspace results. The returned block always carries
 // the caller's original requestedPath.
 func (s *Scope) validate(requestedPath string) *pathBlock {
-	return s.validateAgainstRoots(requestedPath, s.Roots())
+	return withOutsideWorkspaceAccessReason(s.validateAgainstRoots(requestedPath, s.Roots()), requestedPath, SideEffectWrite)
 }
 
 func (s *Scope) validateRead(requestedPath string) *pathBlock {
-	return s.validateAgainstRoots(requestedPath, s.ReadRoots())
+	return withOutsideWorkspaceAccessReason(s.validateAgainstRoots(requestedPath, s.ReadRoots()), requestedPath, SideEffectRead)
+}
+
+func withOutsideWorkspaceAccessReason(block *pathBlock, requestedPath string, sideEffect SideEffect) *pathBlock {
+	if block == nil || block.Code != BlockOutsideWorkspace || !strings.Contains(block.Reason, " is outside the workspace") {
+		return block
+	}
+	switch sideEffect {
+	case SideEffectRead:
+		block.Reason = fmt.Sprintf("Reading %s requires access outside the workspace.", requestedPath)
+	case SideEffectWrite, SideEffectOutOfWorkspace:
+		block.Reason = fmt.Sprintf("Writing to %s requires access outside the workspace. Use /add-dir or --add-dir to allow writes there.", requestedPath)
+	}
+	return block
 }
 
 func (s *Scope) validateAgainstRoots(requestedPath string, roots []string) *pathBlock {