fix(tui): address PR review for incomplete run replay

Use EventRunIncomplete instead of EventError for stalled plan runs so
resume replays a system notice rather than a hard error. Stamp
completedAt when markIncompleteRemaining leaves a terminal plan, tighten
incomplete plan assertions, and add hydration coverage.

CHANGELOG.md

@@ -127,9 +127,6 @@ tagged. Until then, source builds report the version `dev`.
 ## [Unreleased]
 
 ### Added
-- Shared multi-agent skills discovery: when present, `~/.agents/skills` is searched after the primary
-  Zero skills dir (and before plugin skill roots). `zero skills list` / `info` and the runtime `skill`
-  tool share one multi-root discovery path; install/remove/lock still target only the Zero skills directory.
 - `SECURITY.md` with a private vulnerability-reporting path, `CODE_OF_CONDUCT.md`, this changelog, and
   GitHub issue/PR templates.
 - Interactive `/theme` picker: bare `/theme` opens a popup that live-previews each palette as you move

Makefile

@@ -1,7 +1,7 @@
 # Zero build/test/lint targets. AGENTS.md says "Build with `make`" and "Run `make
 # lint` before opening a PR" — these targets back those instructions.
 .DEFAULT_GOAL := build
-.PHONY: build build-all test test-race vet fmt fmt-check lint tidy clean baseline help
+.PHONY: build build-all test test-race vet fmt fmt-check lint tidy clean help
 
 # Build the main CLI binary into ./zero.
 build:
@@ -40,19 +40,5 @@ clean:
 	rm -f zero
 	go clean ./...
 
-# Run the per-turn benchmark harness over the checked-in baseline manifest and
-# write the JSON result to internal/perfbench/reports/baseline.json. Requires a
-# built `zero` binary and a model; set ZERO_BENCH_MODEL (required) and
-# ZERO_BENCH_BINARY (defaults to ./zero) to configure the run. The report is
-# machine-specific and regenerated, not hand-edited.
-baseline: build
-	@if [ -z "$(ZERO_BENCH_MODEL)" ]; then echo "Set ZERO_BENCH_MODEL (and optionally ZERO_BENCH_BINARY) before running 'make baseline'"; exit 2; fi
-	@ZERO_BIN="$${ZERO_BENCH_BINARY:-./zero}"; \
-	go run ./cmd/zero-perf-bench turn \
-		--suite internal/perfbench/manifests/baseline.json \
-		--model $(ZERO_BENCH_MODEL) \
-		--binary "$$ZERO_BIN" \
-		--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, tidy, clean"

README.md

@@ -129,18 +129,11 @@ the key in the wizard:
 export OPENAI_API_KEY=sk-...
 export ANTHROPIC_API_KEY=...
 export GEMINI_API_KEY=...
-export AIMLAPI_API_KEY=...
 export LONGCAT_API_KEY=...
 export MINIMAX_API_KEY=...
 export MINIMAXI_API_KEY=...
 ```
 
-To configure AI/ML API directly, run:
-
-```bash
-zero providers setup aimlapi --set-active
-```
-
 To configure Meituan LongCat (LongCat-2.0) directly, run:
 
 ```bash

cmd/zero-perf-bench/main.go

@@ -39,9 +39,6 @@ func run(args []string, getenv func(string) string, stdout io.Writer, stderr io.
 	if len(args) > 0 && args[0] == "tasks" {
 		return runTasksCommand(args[1:], getenv, stdout, stderr)
 	}
-	if len(args) > 0 && args[0] == "turn" {
-		return runTurnCommand(args[1:], getenv, stdout, stderr)
-	}
 	options, err := parseArgs(args, getenv)
 	if err != nil {
 		_, _ = fmt.Fprintln(stderr, err.Error())
@@ -209,7 +206,6 @@ func helpText() string {
 	return strings.Join([]string{
 		"Usage: zero-perf-bench [options]",
 		"       zero-perf-bench tasks [options]   (Terminal-Bench-style task harness; see `tasks --help`)",
-		"       zero-perf-bench turn  [options]   (per-turn tracing benchmark; see `turn --help`)",
 		"",
 		"Options:",
 		"  --iterations <n>             Measured samples to collect (default: 5)",

cmd/zero-perf-bench/turn.go

@@ -1,238 +0,0 @@
-package main
-
-import (
-	"bytes"
-	"context"
-	"fmt"
-	"io"
-	"os"
-	"path/filepath"
-	"strings"
-
-	"github.com/Gitlawb/zero/internal/perfbench"
-)
-
-// turnOptions configures the `zero-perf-bench turn` subcommand: the per-turn
-// benchmark harness that runs ZERO headlessly with --trace, parses each turn's
-// NDJSON trace, and records per-span latency plus the top controllable latency
-// sources — the Phase 0 baseline's "do not proceed until" criterion.
-type turnOptions struct {
-	SuitePath   string
-	Model       string
-	Mode        string
-	SelfCorrect bool
-	Binary      string
-	Iterations  int
-	Version     string
-	Commit      string
-	Output      string
-	JSON        bool
-	DryRun      bool
-	Help        bool
-}
-
-func runTurnCommand(args []string, getenv func(string) string, stdout io.Writer, stderr io.Writer) int {
-	options, err := parseTurnArgs(args, getenv)
-	if err != nil {
-		_, _ = fmt.Fprintln(stderr, err.Error())
-		return 2
-	}
-	if options.Help {
-		_, _ = fmt.Fprint(stdout, turnHelpText())
-		return 0
-	}
-
-	set, err := perfbench.LoadTaskSet(options.SuitePath)
-	if err != nil {
-		_, _ = fmt.Fprintln(stderr, "[zero] Turn benchmark failed: "+err.Error())
-		return 1
-	}
-
-	// The dry-run path records a zero-iteration run without a binary, so the
-	// manifest loads and the report path is exercised in CI without a model.
-	if options.DryRun {
-		_, _ = fmt.Fprintln(stdout, "[zero] turn benchmark: dry run (no agent invoked)")
-		return 0
-	}
-
-	binary, err := perfbench.ResolveBinary(options.Binary)
-	if err != nil {
-		_, _ = fmt.Fprintln(stderr, "[zero] Turn benchmark failed: "+err.Error())
-		return 2
-	}
-
-	result, err := perfbench.RunTurnBench(context.Background(), set, perfbench.TurnBenchConfig{
-		Model:       options.Model,
-		Mode:        options.Mode,
-		SelfCorrect: options.SelfCorrect,
-		Version:     options.Version,
-		Commit:      options.Commit,
-		Iterations:  options.Iterations,
-		Runner:      perfbench.NewTurnExecRunner(binary),
-	})
-	if err != nil {
-		_, _ = fmt.Fprintln(stderr, "[zero] Turn benchmark failed: "+err.Error())
-		return 1
-	}
-
-	if options.Output != "" {
-		if err := writeTurnReport(options.Output, result); err != nil {
-			_, _ = fmt.Fprintln(stderr, "[zero] Turn benchmark failed: "+err.Error())
-			return 1
-		}
-	}
-	if options.JSON {
-		if err := perfbench.WriteTurnBenchJSON(stdout, result); err != nil {
-			_, _ = fmt.Fprintln(stderr, "[zero] Turn benchmark failed: "+err.Error())
-			return 1
-		}
-		return 0
-	}
-	_, _ = fmt.Fprintln(stdout, perfbench.FormatTurnBenchSummary(result))
-	return 0
-}
-
-func parseTurnArgs(args []string, getenv func(string) string) (turnOptions, error) {
-	options := turnOptions{
-		Iterations: 1,
-		Version:    strings.TrimSpace(getenv("ZERO_BENCH_VERSION")),
-		Commit:     strings.TrimSpace(getenv("ZERO_BENCH_COMMIT")),
-	}
-	for index := 0; index < len(args); index++ {
-		arg := args[index]
-		flag, inlineValue := splitFlagValue(arg)
-		switch flag {
-		case "--suite":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.SuitePath = value
-			index = next
-		case "--model":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Model = value
-			index = next
-		case "--mode":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Mode = value
-			index = next
-		case "--binary":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Binary = value
-			index = next
-		case "--iterations":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			parsed, err := parsePositiveInteger(flag, value)
-			if err != nil {
-				return options, err
-			}
-			options.Iterations = parsed
-			index = next
-		case "--version":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Version = value
-			index = next
-		case "--commit":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Commit = value
-			index = next
-		case "--output":
-			value, next, err := readOptionValue(args, inlineValue, index, flag)
-			if err != nil {
-				return options, err
-			}
-			options.Output = value
-			index = next
-		case "--self-correct":
-			if strings.Contains(arg, "=") {
-				return options, fmt.Errorf("%s does not accept a value", flag)
-			}
-			options.SelfCorrect = true
-		case "--json":
-			if strings.Contains(arg, "=") {
-				return options, fmt.Errorf("%s does not accept a value", flag)
-			}
-			options.JSON = true
-		case "--dry-run":
-			if strings.Contains(arg, "=") {
-				return options, fmt.Errorf("%s does not accept a value", flag)
-			}
-			options.DryRun = true
-		case "-h", "--help":
-			if strings.Contains(arg, "=") {
-				return options, fmt.Errorf("%s does not accept a value", flag)
-			}
-			options.Help = true
-		default:
-			return options, fmt.Errorf("unknown option: %s", arg)
-		}
-	}
-	if options.Help {
-		return options, nil
-	}
-	if strings.TrimSpace(options.SuitePath) == "" {
-		return options, fmt.Errorf("--suite is required")
-	}
-	if strings.TrimSpace(options.Model) == "" && !options.DryRun {
-		return options, fmt.Errorf("--model is required (or pass --dry-run)")
-	}
-	return options, nil
-}
-
-func writeTurnReport(path string, result perfbench.TurnBenchResult) error {
-	if dir := filepath.Dir(path); dir != "" {
-		if err := os.MkdirAll(dir, 0o755); err != nil {
-			return err
-		}
-	}
-	var buffer bytes.Buffer
-	if err := perfbench.WriteTurnBenchJSON(&buffer, result); err != nil {
-		return err
-	}
-	return os.WriteFile(path, buffer.Bytes(), 0o644)
-}
-
-func turnHelpText() string {
-	return strings.Join([]string{
-		"Usage: zero-perf-bench turn [options]",
-		"",
-		"Runs ZERO headlessly with --trace against a per-turn benchmark task set and",
-		"records per-span latency plus the top controllable latency sources (the",
-		"Phase 0 baseline's \"do not proceed until\" criterion). Each task is a fresh",
-		"`zero exec` process, so iterations are cold-start samples; a warm path needs",
-		"an in-process runner (future work).",
-		"",
-		"Options:",
-		"  --suite <path>      Task set JSON file (required)",
-		"  --model <model>     Model to run (required unless --dry-run)",
-		"  --mode <name>       Exec mode preset to apply",
-		"  --self-correct      Enable the post-edit verify-and-correct loop",
-		"  --binary <path>     Path to the `zero` binary (default: zero on PATH / repo root)",
-		"  --iterations <n>    Times to run each task (default: 1)",
-		"  --version <v>       Record the ZERO version (default: $ZERO_BENCH_VERSION)",
-		"  --commit <sha>      Record the ZERO commit (default: $ZERO_BENCH_COMMIT)",
-		"  --output <path>     Write the JSON result to path",
-		"  --json              Print only the JSON result",
-		"  --dry-run           Load the manifest and exit without invoking the agent",
-		"  -h, --help          Show this help",
-	}, "\n") + "\n"
-}

docs/EXTENDING.md

@@ -102,24 +102,14 @@ The full format spec (frontmatter fields, tool scopes, prompt conventions) is in
 
 Skills are markdown instruction packs the agent can pull in on demand. Each skill is a directory containing a `SKILL.md`. Standalone project skill directories are not supported in this version (shared project-wide skills must go in `AGENTS.md` or as a hook). However, project plugins (section 6) may bundle skills, which are merged into the active run.
 
-Discovery roots (earlier wins on name collisions):
-
-1. **Primary Zero dir** — `$ZERO_SKILLS_DIR` if set, else `$XDG_DATA_HOME/zero/skills`, else `~/.local/share/zero/skills/`
-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`.
+Discovery root: `$ZERO_SKILLS_DIR` → `$XDG_DATA_HOME/zero/skills` → `~/.local/share/zero/skills/`. A missing directory is fine — Zero just reports "no skills".
 
 ```text
-~/.local/share/zero/skills/          # primary (install/remove/lock target)
+~/.local/share/zero/skills/
   run-benchmarks/
     SKILL.md
   write-changelog/
     SKILL.md
-
-~/.agents/skills/                    # optional shared multi-agent root
-  shared-review/
-    SKILL.md
 ```
 
 `SKILL.md` format:
@@ -136,9 +126,9 @@ description: Run the project's benchmark suite and summarize the deltas.
 3. Report any regression > 5% with the function name and the previous value.
 ```
 
-Only `name` and `description` are recognized in the frontmatter today. The `name` defaults to the directory name. Within a single skills root, duplicate names are resolved by lexicographic directory order. Across roots, Zero loads the primary directory first, then `~/.agents/skills`, then plugin skill roots; earlier roots win name collisions. Plugin-declared skills (section 6) are merged into the active agent run at plugin activation time, so bundled skills appear in the available skills list and can be loaded with the `skill` tool.
+Only `name` and `description` are recognized in the frontmatter today. The `name` defaults to the directory name. Within a single skills root, duplicate names are resolved by lexicographic directory order. During an agent run, Zero loads the default skills directory before plugin skill roots; earlier roots win name collisions silently. Plugin-declared skills (section 6) are merged into the active agent run at plugin activation time, so bundled skills appear in the available skills list and can be loaded with the `skill` tool.
 
-The `skill` tool lets the agent load any discovered skill by name (primary, agents, or plugin).
+The `skill` core tool lets the agent load any discovered skill by name.
 
 ## 4. Hooks
 
@@ -250,13 +240,10 @@ OAuth server names ending in `.<32 hex chars>` are reserved for token storage.
 
 ```bash
 zero serve --mcp
-zero serve --mcp -C /path/to/workspace
-zero serve --mcp --add-dir /path/to/extra
 ```
 
 The server speaks MCP over stdio. Configure it from the receiving side as a `stdio` server whose command is `zero serve --mcp`.
 
-`-C/--cwd` sets the workspace root exposed as MCP resources and used by core tools. `--add-dir` (repeatable) widens that resource/tool scope beyond the workspace without granting the sandbox temp roots used by interactive runs.
 ## 6. Plugins
 
 A plugin is a self-contained directory that bundles tools, hooks, and skills for one capability. Plugins live at:

docs/HOW_ZERO_WORKS.md

@@ -733,10 +733,9 @@ flowchart TD
 - **Skills** are instruction packs the model can load on demand.
 - **Specialists** are sub-agents callable through the `Task` tool.
 - **MCP servers** contribute external tools.
-- **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.
+- **Plugins** can add tools, hooks, and skill roots. Bootstrap merges plugin skill
+  roots into the runtime skill tool and prompt-visible skill list; the base
+  `internal/skills` scanner still scans one root at a time.
 - **Hooks** can observe or block tool lifecycle events.
 
 ## End-to-End Data Flow

go.mod

@@ -3,9 +3,9 @@ module github.com/Gitlawb/zero
 go 1.26.5
 
 require (
-	charm.land/bubbles/v2 v2.1.1
-	charm.land/bubbletea/v2 v2.0.8
-	charm.land/lipgloss/v2 v2.0.5
+	charm.land/bubbles/v2 v2.1.0
+	charm.land/bubbletea/v2 v2.0.7
+	charm.land/lipgloss/v2 v2.0.4
 	github.com/alecthomas/chroma/v2 v2.27.0
 	github.com/atotto/clipboard v0.1.4
 	github.com/aymanbagabas/go-udiff v0.4.1
@@ -14,18 +14,18 @@ require (
 	github.com/charmbracelet/x/term v0.2.2
 	github.com/coder/websocket v1.8.15
 	github.com/ledongthuc/pdf v0.0.0-20250511090121-5959a4027728
-	golang.org/x/sys v0.47.0
+	golang.org/x/sys v0.46.0
 	mvdan.cc/sh/v3 v3.13.1
 )
 
 require (
-	github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 // indirect
+	github.com/charmbracelet/ultraviolet v0.0.0-20260615092913-2399af76d5b1 // indirect
 	github.com/charmbracelet/x/exp/golden v0.0.0-20260615092313-b57e5e6d29bb // indirect
 	github.com/charmbracelet/x/termios v0.1.1 // indirect
 	github.com/charmbracelet/x/windows v0.2.2 // indirect
 	github.com/clipperhouse/displaywidth v0.11.0 // indirect
 	github.com/clipperhouse/uax29/v2 v2.7.0 // indirect
-	github.com/dlclark/regexp2/v2 v2.5.0 // indirect
+	github.com/dlclark/regexp2/v2 v2.2.1 // indirect
 	github.com/go-quicktest/qt v1.102.0 // indirect
 	github.com/lucasb-eyer/go-colorful v1.4.0 // indirect
 	github.com/mattn/go-runewidth v0.0.24 // indirect
@@ -34,5 +34,5 @@ require (
 	github.com/rogpeppe/go-internal v1.15.0 // indirect
 	github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e // indirect
 	golang.org/x/exp v0.0.0-20260611194520-c48552f49976 // indirect
-	golang.org/x/sync v0.22.0 // indirect
+	golang.org/x/sync v0.21.0 // indirect
 )

go.sum

@@ -1,9 +1,9 @@
-charm.land/bubbles/v2 v2.1.1 h1:7r55WzBxpo/R3z98hGmY7KKPd3ET6vsf0Fb9sDHOV60=
-charm.land/bubbles/v2 v2.1.1/go.mod h1:GE6M31gaWZVXzGw73OeuTTgy4lX+OtkH0E5ymnNsHxo=
-charm.land/bubbletea/v2 v2.0.8 h1:SxTJMhCAI3lbPmy4SgX5LWZ24AdINr4I6UEqzZvYJuY=
-charm.land/bubbletea/v2 v2.0.8/go.mod h1:2SkdgoTXluXJHOUwAoRlRXF/28vklb1rFl6GcgV1/ss=
-charm.land/lipgloss/v2 v2.0.5 h1:kbNxgeeUOYv5J0YdpxFjfvf3dFvqH8Aci4zB6xqFtrY=
-charm.land/lipgloss/v2 v2.0.5/go.mod h1:9oqhxt4yxIMe6q5A4kHr44DremZk7J9UNh74GlWa5nc=
+charm.land/bubbles/v2 v2.1.0 h1:YSnNh5cPYlYjPxRrzs5VEn3vwhtEn3jVGRBT3M7/I0g=
+charm.land/bubbles/v2 v2.1.0/go.mod h1:l97h4hym2hvWBVfmJDtrEHHCtkIKeTEb3TTJ4ZOB3wY=
+charm.land/bubbletea/v2 v2.0.7 h1:7qw2tTAVar7m7klOPBYfTB0mniv/RuexsYwMRNxSeL0=
+charm.land/bubbletea/v2 v2.0.7/go.mod h1:DGW2q8gvzHnOpMpZTORs0aySVHCox5C+2Svk0fci1qs=
+charm.land/lipgloss/v2 v2.0.4 h1:lcPeVtcp23SNra7lHy8iYE4UC2aIipVQ47sbGyyxR5Q=
+charm.land/lipgloss/v2 v2.0.4/go.mod h1:0653x8epbZSzdDfO/XPS1a/uYPOBeSsCssOpJOqDzik=
 github.com/alecthomas/assert/v2 v2.11.0 h1:2Q9r3ki8+JYXvGsDyBXwH3LcJ+WK5D0gc5E8vS6K3D0=
 github.com/alecthomas/assert/v2 v2.11.0/go.mod h1:Bze95FyfUr7x34QZrjL+XP+0qgp/zg8yS+TtBj1WA3k=
 github.com/alecthomas/chroma/v2 v2.27.0 h1:FodwmyOBgJULFYmDqibcp9pvfDLWdtPRh9v/r5BXYZs=
@@ -16,8 +16,8 @@ github.com/aymanbagabas/go-udiff v0.4.1 h1:OEIrQ8maEeDBXQDoGCbbTTXYJMYRCRO1fnodZ
 github.com/aymanbagabas/go-udiff v0.4.1/go.mod h1:0L9PGwj20lrtmEMeyw4WKJ/TMyDtvAoK9bf2u/mNo3w=
 github.com/charmbracelet/colorprofile v0.4.3 h1:QPa1IWkYI+AOB+fE+mg/5/4HRMZcaXex9t5KX76i20Q=
 github.com/charmbracelet/colorprofile v0.4.3/go.mod h1:/zT4BhpD5aGFpqQQqw7a+VtHCzu+zrQtt1zhMt9mR4Q=
-github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7 h1:3FmWoGNWK4STvqg0O0Aeav2T7rodWJAPeF0QpH+8gFw=
-github.com/charmbracelet/ultraviolet v0.0.0-20260703014108-f5a850f9c2b7/go.mod h1:f/jRa757WUmaOZrbPspXymbg/GnbF+rwe4OLsG7aXYo=
+github.com/charmbracelet/ultraviolet v0.0.0-20260615092913-2399af76d5b1 h1:4+r3uOJ69ueRBt4okgEfWZeXs3BD36HcDBmOIAUlETk=
+github.com/charmbracelet/ultraviolet v0.0.0-20260615092913-2399af76d5b1/go.mod h1:f/jRa757WUmaOZrbPspXymbg/GnbF+rwe4OLsG7aXYo=
 github.com/charmbracelet/x/ansi v0.11.7 h1:kzv1kJvjg2S3r9KHo8hDdHFQLEqn4RBCb39dAYC84jI=
 github.com/charmbracelet/x/ansi v0.11.7/go.mod h1:9qGpnAVYz+8ACONkZBUWPtL7lulP9No6p1epAihUZwQ=
 github.com/charmbracelet/x/exp/golden v0.0.0-20260615092313-b57e5e6d29bb h1:m1Uub9uM7meZ8ofVyeRfK7M/knCi3sF2gKn9kldVW0I=
@@ -34,8 +34,8 @@ github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJ
 github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM=
 github.com/coder/websocket v1.8.15 h1:6B2JPeOGlpff2Uz6vOEH1Vzpi0iUz20A+lPVhPHtNUA=
 github.com/coder/websocket v1.8.15/go.mod h1:NX3SzP+inril6yawo5CQXx8+fk145lPDC6pumgx0mVg=
-github.com/dlclark/regexp2/v2 v2.5.0 h1:liNiWIPCvCh5HBcYfsjd+P16AG79fwd6T1Toy2gOtEA=
-github.com/dlclark/regexp2/v2 v2.5.0/go.mod h1:avUrQvPaLz2DrFNHJF0taWAFFX2C1GMSSoeiqFjcBmU=
+github.com/dlclark/regexp2/v2 v2.2.1 h1:mf4KkFUj0gJuarK8P+LgiS+Lit7m9N1yAwEfPbee7R0=
+github.com/dlclark/regexp2/v2 v2.2.1/go.mod h1:avUrQvPaLz2DrFNHJF0taWAFFX2C1GMSSoeiqFjcBmU=
 github.com/go-quicktest/qt v1.102.0 h1:HSQxCeh5YZH3EL3W39ixjtyaEhcWSXQHtHnMBzSs474=
 github.com/go-quicktest/qt v1.102.0/go.mod h1:p4lGIVX+8Wa6ZPNDvqcxq36XpUDLh42FLetFU7odllI=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=
@@ -62,9 +62,9 @@ github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e h1:JVG44RsyaB9T2KIHavM
 github.com/xo/terminfo v0.0.0-20220910002029-abceb7e1c41e/go.mod h1:RbqR21r5mrJuqunuUZ/Dhy/avygyECGrLceyNeo4LiM=
 golang.org/x/exp v0.0.0-20260611194520-c48552f49976 h1:X8Hz2ImujgbmetVuW+w2YkyZChE3cBpZi2P158rTG9M=
 golang.org/x/exp v0.0.0-20260611194520-c48552f49976/go.mod h1:vnf4pv9iKZXY58sQE1L86zmNWJ4159e1RkcWiLCkeEY=
-golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
-golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
-golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs=
-golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
+golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM=
+golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
+golang.org/x/sys v0.46.0 h1:noSf2Fq6F8DBgS+LysIkx7rIExoNHJsxOAtPp4rthXw=
+golang.org/x/sys v0.46.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw=
 mvdan.cc/sh/v3 v3.13.1 h1:DP3TfgZhDkT7lerUdnp6PTGKyxxzz6T+cOlY/xEvfWk=
 mvdan.cc/sh/v3 v3.13.1/go.mod h1:lXJ8SexMvEVcHCoDvAGLZgFJ9Wsm2sulmoNEXGhYZD0=

internal/agent/compaction.go

@@ -6,7 +6,6 @@ import (
 	"errors"
 	"strings"
 
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -431,12 +430,6 @@ func (state *compactionState) maybeCompact(
 		state.lowWaterMark = size
 		return messages
 	}
-	// Only count a compaction when it actually shrank the history, so the
-	// compaction counter reflects real context reductions rather than paid
-	// no-ops that left the token budget untouched.
-	if r := trace.FromContext(ctx); r != nil {
-		r.Counter(trace.CounterCompactionCount, 1)
-	}
 	state.lowWaterMark = newSize
 	return compacted
 }
@@ -486,11 +479,7 @@ func (state *compactionState) recover(
 	// one-shot budget now so a provider that keeps returning context-limit errors
 	// after a successful compaction can't loop forever. Store the low-water mark in
 	// the SAME combined (messages + tool-defs) domain maybeCompact uses, so the
-	// proactive shrink-guard compares like with like. Count it now that the shrink
-	// is confirmed, so the counter mirrors maybeCompact's real-reduction policy.
-	if r := trace.FromContext(ctx); r != nil {
-		r.Counter(trace.CounterCompactionCount, 1)
-	}
+	// proactive shrink-guard compares like with like.
 	state.reactiveAttempted = true
 	state.lowWaterMark = state.calibratedTokens(estimateTokens(result) + estimateToolDefTokens(tools))
 	return result, true, nil

internal/agent/completion_gate_test.go

@@ -2,6 +2,7 @@ package agent
 
 import (
 	"context"
+	"errors"
 	"strings"
 	"testing"
 
@@ -126,9 +127,107 @@ func TestCompletionGateContinuesOnCueThenSucceeds(t *testing.T) {
 	}
 }
 
-// With the gate OFF (the interactive/TUI default), a continuation-cue turn is
-// accepted as the final answer exactly as before — guaranteeing no behavior
-// change for non-headless callers.
+// Issue #666: the plan-aware gate applies even when RequireCompletionSignal is
+// off (interactive/TUI). A model that creates a plan then stalls on a
+// continuation-cue text turn must be re-prompted and eventually INCOMPLETE, not
+// accepted as success while the plan panel stays on step one.
+func TestPlanPendingGateAppliesWithoutRequireCompletionSignal(t *testing.T) {
+	cue := "Now I need to configure the SSH server. Let me check the current SSH configuration:"
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending", "pending"),
+		textTurn(cue), textTurn(cue), textTurn(cue), textTurn(cue), textTurn(cue),
+	}}
+
+	result, err := Run(context.Background(), "set up a git server", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+		// RequireCompletionSignal deliberately off — interactive/TUI default.
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true (plan stalled without headless gate), got false; final=%q turns=%d", result.FinalAnswer, result.Turns)
+	}
+	if len(provider.requests) != 1+maxContinueNudges+1 {
+		t.Fatalf("expected %d provider turns (1 plan + %d nudges + 1 final), got %d",
+			1+maxContinueNudges+1, maxContinueNudges, len(provider.requests))
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected a continue nudge (%q) to be injected", continueNudgeMarker)
+	}
+}
+
+// A pending plan must nudge non-colon continuation announcements too — not only
+// colon-terminated cues — so the first mid-step text turn cannot finalize while
+// steps remain. A later clean final answer is accepted after the bounded nudges.
+func TestPlanPendingNudgesNonColonContinuationText(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	midStep := "Let me inspect the configuration."
+	final := "Provider profiles are loaded from config and merged with defaults."
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending", "pending"),
+		textTurn(midStep),
+		textTurn(final), textTurn(final), textTurn(final), textTurn(final),
+	}}
+
+	result, err := Run(context.Background(), "inspect and configure", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("expected success after nudge + final answer, got Incomplete (%q)", result.IncompleteReason)
+	}
+	if result.FinalAnswer != final {
+		t.Fatalf("final answer = %q, want %q", result.FinalAnswer, final)
+	}
+	if len(provider.requests) != 1+1+maxContinueNudges {
+		t.Fatalf("expected %d turns (plan + mid-step + %d nudged finals), got %d",
+			1+1+maxContinueNudges, maxContinueNudges, len(provider.requests))
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected a continue nudge after the mid-step text with pending plan")
+	}
+}
+
+// After bounded nudges, a confident final answer with stale plan bookkeeping must
+// still succeed on the interactive/TUI path (no infinite loop, no false incomplete).
+func TestPlanPendingAcceptsFinalAnswerAfterBoundedNudgesWithoutHeadlessGate(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	done := "All provider profiles are documented above."
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		planTurn("in_progress", "pending"),
+		textTurn(done), textTurn(done), textTurn(done), textTurn(done),
+	}}
+
+	result, err := Run(context.Background(), "explain provider loading", provider, Options{
+		Registry: registry,
+		MaxTurns: 10,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("stale plan alone must not force Incomplete on TUI path; reason=%q", result.IncompleteReason)
+	}
+	if result.FinalAnswer != done {
+		t.Fatalf("final answer = %q, want %q", result.FinalAnswer, done)
+	}
+	if !someRequestContains(provider.requests, continueNudgeMarker) {
+		t.Fatalf("expected at least one continue nudge before accepting completion")
+	}
+}
+
+// With the gate OFF and NO pending plan, a continuation-cue turn is still
+// accepted as the final answer — no behavior change for short single-step tasks.
 func TestCompletionGateOffPreservesLegacyBehavior(t *testing.T) {
 	cue := "Let me check the config:"
 	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
@@ -177,6 +276,44 @@ func TestContinuationCueMatching(t *testing.T) {
 	}
 }
 
+// Cancellation during a plan-pending stall must return immediately without extra
+// nudges or incomplete synthesis.
+func TestRunCancellationDuringPlanPendingStopsImmediately(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	ctx, cancel := context.WithCancel(context.Background())
+	_, err := Run(ctx, "do work", cancelMidStreamProvider{cancel: cancel}, Options{
+		Registry: registry,
+	})
+	if !errors.Is(err, context.Canceled) {
+		t.Fatalf("expected context.Canceled, got %v", err)
+	}
+}
+
+// Repeated identical update_plan calls must still hit the max-turns ceiling rather
+// than looping silently forever.
+func TestRunRepeatedIdenticalUpdatePlanHitsMaxTurns(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+	same := toolTurn("plan", "update_plan", `{"plan":[{"content":"step","status":"in_progress"}]}`)
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{same, same, same, textTurn("halted summary")}}
+
+	result, err := Run(context.Background(), "keep planning", provider, Options{
+		Registry: registry,
+		MaxTurns: 3,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	// MaxTurns=3 tool turns, then finalAnswerAfterMaxTurns issues one more request.
+	if len(provider.requests) != 4 {
+		t.Fatalf("expected 3 tool turns + 1 max-turns final-answer call, got %d", len(provider.requests))
+	}
+	if result.FinalAnswer == "" {
+		t.Fatal("expected a max-turns final answer")
+	}
+}
+
 // review #4: a run that loops to the MaxTurns ceiling (always calling a tool, so it
 // never reaches the no-tool-call gate) was reported as success. Under the headless
 // gate, a max-turns cutoff is INCOMPLETE — the agent was stopped mid-run, not done.

internal/agent/guardrails.go

@@ -508,9 +508,19 @@ func (state *guardState) observeTurn(collected zeroruntime.CollectedStream) (sto
 	hasVisibleText := strings.TrimSpace(collected.Text) != ""
 	hasReasoning := collected.HasReasoning || len(collected.ReasoningBlocks) > 0
 
-	if hasToolCalls || hasVisibleText || hasReasoning {
+	productive := hasToolCalls || hasVisibleText
+	switch {
+	case productive:
 		state.emptyTurns = 0
-	} else {
+	case hasReasoning && state.planItemsPending > 0:
+		// Reasoning alone does not prove progress while plan steps remain — weaker
+		// models (e.g. HY3 via OpenAI-compatible gateways) can stream long
+		// reasoning-only turns without tools or visible text, burning the turn
+		// budget while the plan panel stays on the first in_progress step.
+		state.emptyTurns++
+	case hasReasoning:
+		state.emptyTurns = 0
+	default:
 		state.emptyTurns++
 	}
 	if hasToolCalls && !hasVisibleText {

internal/agent/guardrails_test.go

@@ -30,6 +30,16 @@ func reasoningTurn(content string) []zeroruntime.StreamEvent {
 	}
 }
 
+// multiReasoningTurn streams several reasoning chunks inside ONE provider turn.
+func multiReasoningTurn(chunks ...string) []zeroruntime.StreamEvent {
+	events := make([]zeroruntime.StreamEvent, 0, len(chunks)+1)
+	for _, chunk := range chunks {
+		events = append(events, zeroruntime.StreamEvent{Type: zeroruntime.StreamEventReasoning, Content: chunk})
+	}
+	events = append(events, zeroruntime.StreamEvent{Type: zeroruntime.StreamEventDone})
+	return events
+}
+
 // toolTurn produces a turn that calls a named tool with the given args JSON.
 func toolTurn(callID string, toolName string, args string) []zeroruntime.StreamEvent {
 	return []zeroruntime.StreamEvent{
@@ -109,6 +119,95 @@ func TestRunResetsEmptyTurnCounterOnVisibleOutput(t *testing.T) {
 	}
 }
 
+func TestRunStopsReasoningOnlyTurnsWhilePlanPending(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		reasoningTurn("thinking 1"),
+		reasoningTurn("thinking 2"),
+		reasoningTurn("thinking 3"),
+		textTurn("should never reach here"),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(provider.requests) != maxEmptyTurns+1 {
+		t.Fatalf("expected %d turns (1 plan + %d reasoning-only stalls), got %d", maxEmptyTurns+1, maxEmptyTurns, len(provider.requests))
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true after reasoning-only stall with pending plan, got false")
+	}
+	if !strings.Contains(result.FinalAnswer, noOutputStopMarker) {
+		t.Fatalf("expected no-output stop answer, got %q", result.FinalAnswer)
+	}
+}
+
+func TestRunMultiChunkReasoningCountsAsOneNonProductiveTurnWhilePlanPending(t *testing.T) {
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		multiReasoningTurn("chunk-1 ", "chunk-2 ", "chunk-3 ", "chunk-4 ", "chunk-5 "),
+		multiReasoningTurn("more-1 ", "more-2 ", "more-3 "),
+		multiReasoningTurn("final-1 ", "final-2 "),
+		textTurn("should never reach here"),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(provider.requests) != maxEmptyTurns+1 {
+		t.Fatalf("expected %d turns (1 plan + %d multi-chunk reasoning stalls), got %d",
+			maxEmptyTurns+1, maxEmptyTurns, len(provider.requests))
+	}
+	if !result.Incomplete {
+		t.Fatalf("expected Incomplete=true, got false")
+	}
+}
+
+func TestRunReasoningThenToolCallSucceedsWithPendingPlan(t *testing.T) {
+	root := t.TempDir()
+	writeAgentTestFile(t, root+"/notes.txt", "alpha")
+	registry := tools.NewRegistry()
+	registry.Register(tools.NewReadFileTool(root))
+	registry.Register(tools.NewUpdatePlanTool())
+
+	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{
+		toolTurn("plan", "update_plan", `{"plan":[{"content":"step 1","status":"in_progress"},{"content":"step 2","status":"pending"}]}`),
+		multiReasoningTurn("thinking ", "more thinking "),
+		toolTurn("read", "read_file", `{"path":"notes.txt"}`),
+		toolTurn("plan2", "update_plan", `{"plan":[{"content":"step 1","status":"completed"},{"content":"step 2","status":"completed"}]}`),
+		textTurn("Both steps are done."),
+	}}
+
+	result, err := Run(context.Background(), "multi-step task", provider, Options{
+		Registry: registry,
+		MaxTurns: 12,
+		Cwd:      root,
+	})
+	if err != nil {
+		t.Fatal(err)
+	}
+	if result.Incomplete {
+		t.Fatalf("run should succeed after productive tool call, got Incomplete (%q)", result.IncompleteReason)
+	}
+	if result.FinalAnswer != "Both steps are done." {
+		t.Fatalf("final answer = %q", result.FinalAnswer)
+	}
+}
+
 func TestRunResetsEmptyTurnCounterOnReasoning(t *testing.T) {
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
@@ -293,7 +392,7 @@ func TestRunDoesNotInjectNotCalledReminderWhenPlanUsed(t *testing.T) {
 
 	provider := &mockProvider{
 		turns: [][]zeroruntime.StreamEvent{
-			toolTurn("call-1", "update_plan", `{"plan":[{"content":"step one"}]}`),
+			toolTurn("call-1", "update_plan", `{"plan":[{"content":"step one","status":"completed"}]}`),
 			toolTurn("call-2", "read_file", `{"path":"notes.txt"}`),
 			textTurn("done"),
 		},
@@ -329,6 +428,12 @@ func TestRunInjectsStalePlanReminderAfterManyToolCalls(t *testing.T) {
 	for i := 0; i < staleToolCallThreshold+2; i++ {
 		turns = append(turns, toolTurn("call", "read_file", `{"path":"notes.txt"}`))
 	}
+	// The plan-aware completion gate re-prompts up to maxContinueNudges when the
+	// model stops with text while plan items remain; supply those turns before
+	// the final answer.
+	for i := 0; i < maxContinueNudges; i++ {
+		turns = append(turns, textTurn("done."))
+	}
 	turns = append(turns, textTurn("done"))
 
 	provider := &mockProvider{turns: turns}
@@ -363,6 +468,9 @@ func TestRunStalePlanReminderIsOneShotPerInterval(t *testing.T) {
 	for i := 0; i < staleToolCallThreshold*2; i++ {
 		turns = append(turns, toolTurn("call", "read_file", `{"path":"notes.txt"}`))
 	}
+	for i := 0; i < maxContinueNudges; i++ {
+		turns = append(turns, textTurn("done."))
+	}
 	turns = append(turns, textTurn("done"))
 
 	provider := &mockProvider{turns: turns}

internal/agent/loop.go

@@ -15,7 +15,6 @@ import (
 	"github.com/Gitlawb/zero/internal/sandbox"
 	"github.com/Gitlawb/zero/internal/streamjson"
 	"github.com/Gitlawb/zero/internal/tools"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -115,26 +114,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 		return Result{}, errors.New("agent provider is required")
 	}
 
-	// Tracing is opt-in. When a recorder is wired, thread it into ctx so the
-	// providerio seam and reconnect helper can reach it via trace.FromContext,
-	// mark the run's start, and wrap OnUsage so token counters accumulate for
-	// free alongside the existing per-request plumbing. nil recorder leaves the
-	// loop byte-identical: FromContext returns nil, the no-op stamps cost
-	// nothing, and the OnUsage wrapper is not installed.
-	if options.Trace != nil {
-		ctx = trace.WithContext(ctx, options.Trace)
-		options.Trace.Start()
-		originalOnUsage := options.OnUsage
-		options.OnUsage = func(usage Usage) {
-			if originalOnUsage != nil {
-				originalOnUsage(usage)
-			}
-			options.Trace.Counter(trace.CounterInputTokens, int64(usage.InputTokens))
-			options.Trace.Counter(trace.CounterCachedInputTokens, int64(usage.CachedInputTokens))
-			options.Trace.Counter(trace.CounterOutputTokens, int64(usage.OutputTokens))
-		}
-	}
-
 	maxTurns := options.MaxTurns
 	if maxTurns <= 0 {
 		maxTurns = 12
@@ -205,20 +184,12 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 		// 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
-		// the messages, so computing it before compaction is safe.
-		toolPartitionSpan := options.Trace.Span(trace.SpanToolPartition)
 		exposed, _ := partitionToolsCached(registry, permissionMode, options, loaded, toolDefCache)
-		toolPartitionSpan.End()
 
 		// PROACTIVE compaction: if the history is approaching the model's
 		// context window, summarize the oldest middle before building the
 		// request. A no-op when ContextWindow == 0 (compaction disabled).
-		compactionSpan := options.Trace.Span(trace.SpanCompaction)
 		messages = compactor.maybeCompact(ctx, provider, messages, exposed)
-		compactionSpan.End()
 		request := zeroruntime.CompletionRequest{
 			Messages:        copyMessages(messages),
 			Tools:           exposed,
@@ -284,39 +255,11 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 		// conversation-state duplication.
 		forwardedVisibleText := false
 		forwardingOpts := zeroruntime.CollectOptions{OnUsage: options.OnUsage}
-		// Install text/reasoning forwarding handlers whenever EITHER a user
-		// callback OR a trace recorder is set. A headless traced run (e.g. `zero
-		// exec --trace`) sets Trace but no OnText/OnReasoning; without these
-		// handlers the stream is still collected, but FirstTokenAt would never
-		// stamp and the trace would lose its TTFT signal. The trace recorder's
-		// stamp methods are nil-safe, but we guard on `trace != nil` so a run with
-		// a user callback but no recorder still works. forwardedVisibleText stays
-		// tied to the USER callback only, preserving the stall-retry semantics
-		// (a trace-only handler forwards nothing the user would see duplicated).
-		rec := options.Trace
-		onText := options.OnText
-		if onText != nil || rec != nil {
-			forwardingOpts.OnText = func(s string) {
-				if rec != nil {
-					rec.StampFirstVisibleEvent()
-					rec.StampFirstToken()
-				}
-				if onText != nil {
-					forwardedVisibleText = true
-					onText(s)
-				}
-			}
+		if options.OnText != nil {
+			forwardingOpts.OnText = func(s string) { forwardedVisibleText = true; options.OnText(s) }
 		}
-		onReasoning := options.OnReasoning
-		if onReasoning != nil || rec != nil {
-			forwardingOpts.OnReasoning = func(s string) {
-				if rec != nil {
-					rec.StampFirstToken()
-				}
-				if onReasoning != nil {
-					onReasoning(s)
-				}
-			}
+		if options.OnReasoning != nil {
+			forwardingOpts.OnReasoning = func(s string) { options.OnReasoning(s) }
 		}
 		if options.OnToolCallStart != nil {
 			forwardingOpts.OnToolCallStart = func(id, name string) { options.OnToolCallStart(id, name) }
@@ -354,18 +297,14 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				if retryStreamErr != nil {
 					return collected, retryStreamErr
 				}
-				genSpan := options.Trace.Span(trace.SpanGeneration)
 				collected = zeroruntime.CollectStreamWithOptions(ctx, retryStream, zeroruntime.CollectOptions{
 					OnUsage: options.OnUsage,
 				})
-				genSpan.End()
 			}
 			return collected, nil
 		}
 
-		generationSpan := options.Trace.Span(trace.SpanGeneration)
 		collected := zeroruntime.CollectStreamWithOptions(ctx, stream, forwardingOpts)
-		generationSpan.End()
 		if collected.Error != "" {
 			updated, stop := recoverStreamError(collected)
 			collected = updated
@@ -418,9 +357,7 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				result.Messages = copyMessages(messages)
 				return result, retryErr
 			}
-			stallGenSpan := options.Trace.Span(trace.SpanGeneration)
 			collected = zeroruntime.CollectStreamWithOptions(ctx, retryStream, forwardingOpts)
-			stallGenSpan.End()
 		}
 		if collected.Error != "" {
 			// Route a reissued stream's non-stall error through the SAME recovery as
@@ -479,6 +416,10 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 			// counted toward the runaway cap so we stop before burning maxTurns.
 			if guards.observeTurn(collected) {
 				result.FinalAnswer = noOutputStopAnswer(result.Turns)
+				if guards.pendingPlanItems() {
+					result.Incomplete = true
+					result.IncompleteReason = "stopped after non-productive turns while plan items remained"
+				}
 				result.Messages = copyMessages(messages)
 				return result, nil
 			}
@@ -492,10 +433,13 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				})
 				continue
 			}
-			// Completion gate (headless): a turn with text but no tool call is the
-			// model's final answer ONLY when the work is actually done. Default off
-			// (RequireCompletionSignal), so interactive runs stay byte-identical.
-			if options.RequireCompletionSignal {
+			// Completion gate: a turn with text but no tool call is the model's
+			// final answer ONLY when the work is actually done. Headless exec opts in
+			// via RequireCompletionSignal; interactive runs also apply the gate while
+			// update_plan still has pending/in_progress items so a plan cannot stall
+			// silently after the first step is marked active.
+			planPending := guards.pendingPlanItems()
+			if options.RequireCompletionSignal || planPending {
 				// (1) Self-report downgrade (strongest, unambiguous): the model's own
 				// final message admits it guessed / could not meet the objective. Checked
 				// FIRST so an admitted-impossible task is downgraded immediately (no wasted
@@ -518,11 +462,9 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				// false-fail a completed run with stale bookkeeping) — fall through to the
 				// acceptance check / success.
 				cue := endsWithContinuationCue(collected.Text)
-				planPending := guards.pendingPlanItems()
 				if cue || planPending {
 					if continueNudges < maxContinueNudges {
 						continueNudges++
-						options.Trace.Counter(trace.CounterCompletionNudges, 1)
 						reason := "your message ended mid-step"
 						if !cue {
 							reason = "pending plan items remain — finish them, or mark them complete with update_plan if you are done"
@@ -552,7 +494,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				// (no admission, no cue) then finalizes as success on the next turn.
 				if options.SelfCorrect != nil && !acceptanceRequested {
 					acceptanceRequested = true
-					options.Trace.Counter(trace.CounterAcceptanceChecks, 1)
 					messages = append(messages, zeroruntime.Message{
 						Role:    zeroruntime.MessageRoleUser,
 						Content: acceptanceVerificationNudge(),
@@ -613,26 +554,20 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 					runEnd++
 				}
 				if runEnd-index >= 2 {
-					batchSpan := options.Trace.Span(trace.SpanToolExecution)
 					precomputed = executeParallelReadBatch(ctx, registry, collected.ToolCalls, index, runEnd, permissionMode, options)
-					batchSpan.End()
 					precomputedStart, precomputedEnd = index, runEnd
 				}
 			}
 			if options.OnToolCall != nil {
 				options.OnToolCall(call)
 			}
-			options.Trace.StampFirstUsefulAction()
 			var toolResult ToolResult
 			var abortErr error
 			if index >= precomputedStart && index < precomputedEnd {
 				toolResult, abortErr = precomputed[index-precomputedStart].result, precomputed[index-precomputedStart].abortErr
 			} else {
-				toolSpan := options.Trace.Span(trace.SpanToolExecution)
 				toolResult, abortErr = executeToolCall(ctx, registry, call, permissionMode, options)
-				toolSpan.End()
 			}
-			options.Trace.Counter(trace.CounterToolCalls, 1)
 			if options.OnToolResult != nil {
 				options.OnToolResult(toolResult)
 			}
@@ -737,7 +672,6 @@ func Run(ctx context.Context, prompt string, provider Provider, options Options)
 				// context-window sizing, and usage attribution follow the new model.
 				provider = newProvider
 				options.Model = turnRequestedModel
-				options.Trace.Counter(trace.CounterModelSwitches, 1)
 				// KNOWN LIMITATION (deferred): the compactor's context-window budget
 				// is fixed at run start from options.ContextWindow and is NOT updated
 				// here, so a switch to a model with a different window keeps compacting
@@ -836,7 +770,6 @@ func finalAnswerAfterMaxTurns(ctx context.Context, provider Provider, messages [
 	if err != nil {
 		return "", messages, ""
 	}
-	finalGenSpan := options.Trace.Span(trace.SpanGeneration)
 	collected := zeroruntime.CollectStreamWithOptions(ctx, stream, zeroruntime.CollectOptions{
 		OnText:          options.OnText,
 		OnReasoning:     options.OnReasoning,
@@ -844,7 +777,6 @@ func finalAnswerAfterMaxTurns(ctx context.Context, provider Provider, messages [
 		OnToolCallStart: options.OnToolCallStart,
 		OnToolCallDelta: options.OnToolCallDelta,
 	})
-	finalGenSpan.End()
 	if ctx.Err() != nil || collected.Error != "" || strings.TrimSpace(collected.Text) == "" {
 		return "", messages, ""
 	}
@@ -1948,10 +1880,7 @@ func requestPermission(ctx context.Context, request PermissionRequest, options O
 	if options.OnPermissionRequest == nil {
 		return PermissionDecision{Action: PermissionDecisionDeny, Reason: request.Reason}, nil
 	}
-	permSpan := options.Trace.Span(trace.SpanPermissionWait)
-	decision, err := options.OnPermissionRequest(ctx, request)
-	permSpan.End()
-	return decision, err
+	return options.OnPermissionRequest(ctx, request)
 }
 
 func normalizePermissionDecisionAction(action PermissionDecisionAction) PermissionDecisionAction {

internal/agent/loop_test.go

@@ -15,7 +15,6 @@ import (
 	"github.com/Gitlawb/zero/internal/sandbox"
 	"github.com/Gitlawb/zero/internal/specmode"
 	"github.com/Gitlawb/zero/internal/tools"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -3462,73 +3461,3 @@ func TestRunDoesNotFlagCleanToolOutput(t *testing.T) {
 		t.Errorf("clean output should not get a reminder, got %q", captured.Output)
 	}
 }
-
-// TestRunTracingWrapperStampsUsage verifies the per-turn tracing setup in Run:
-// a wired recorder is Started, OnUsage is wrapped so it still forwards to the
-// caller's callback AND stamps token counters, and the run completes.
-func TestRunTracingWrapperStampsUsage(t *testing.T) {
-	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{{
-		{Type: zeroruntime.StreamEventUsage, Usage: zeroruntime.Usage{InputTokens: 100, CachedInputTokens: 20, OutputTokens: 40}},
-		{Type: zeroruntime.StreamEventText, Content: "done"},
-		{Type: zeroruntime.StreamEventDone},
-	}}}
-	onUsageCalls := 0
-	rec := trace.NewRecorder("tracing-session", "run-1", "test")
-	if _, err := Run(context.Background(), "hi", provider, Options{
-		SessionID:    "tracing-session",
-		Cwd:          t.TempDir(),
-		ProviderName: "test-provider",
-		Model:        "test-model",
-		Trace:        rec,
-		OnUsage:      func(Usage) { onUsageCalls++ },
-	}); err != nil {
-		t.Fatalf("Run: %v", err)
-	}
-	tr := rec.Finish()
-	if tr.StartedAt.IsZero() {
-		t.Fatal("tracing wrapper did not Start the recorder")
-	}
-	// FirstTokenAt must stamp even though no OnText/OnReasoning user callback is
-	// set: a headless traced run (e.g. `zero exec --trace`) sets Trace but no UI
-	// callbacks, so the loop installs trace-only forwarding handlers to capture
-	// TTFT. Without them FirstTokenAt stays zero and the trace loses its signal.
-	if tr.FirstTokenAt.IsZero() {
-		t.Fatal("FirstTokenAt not stamped for a traced run with no OnText/OnReasoning callbacks")
-	}
-	if got := tr.Counter(trace.CounterInputTokens); got != 100 {
-		t.Fatalf("input token counter = %d, want 100", got)
-	}
-	if got := tr.Counter(trace.CounterCachedInputTokens); got != 20 {
-		t.Fatalf("cached input token counter = %d, want 20", got)
-	}
-	if got := tr.Counter(trace.CounterOutputTokens); got != 40 {
-		t.Fatalf("output token counter = %d, want 40", got)
-	}
-	if onUsageCalls == 0 {
-		t.Fatal("wrapped OnUsage did not forward to the caller's callback")
-	}
-}
-
-// TestRunNilTraceForwardsUsage verifies a nil recorder leaves the loop
-// byte-identical: OnUsage is not wrapped, so the caller's callback still fires
-// and nothing panics.
-func TestRunNilTraceForwardsUsage(t *testing.T) {
-	provider := &mockProvider{turns: [][]zeroruntime.StreamEvent{{
-		{Type: zeroruntime.StreamEventUsage, Usage: zeroruntime.Usage{InputTokens: 7}},
-		{Type: zeroruntime.StreamEventText, Content: "done"},
-		{Type: zeroruntime.StreamEventDone},
-	}}}
-	onUsageCalls := 0
-	if _, err := Run(context.Background(), "hi", provider, Options{
-		SessionID:    "nil-trace-session",
-		Cwd:          t.TempDir(),
-		ProviderName: "test-provider",
-		Model:        "test-model",
-		OnUsage:      func(Usage) { onUsageCalls++ },
-	}); err != nil {
-		t.Fatalf("Run: %v", err)
-	}
-	if onUsageCalls == 0 {
-		t.Fatal("OnUsage not forwarded when Trace is nil")
-	}
-}

internal/agent/reconnect.go

@@ -8,7 +8,6 @@ import (
 	"time"
 
 	"github.com/Gitlawb/zero/internal/errhint"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -75,21 +74,14 @@ func stallRetryNoticeFor(options Options) reconnectNotifier {
 // already past its deadline is returned immediately (no retry) — those have
 // their own handling (compaction for context-limit, image-rejection, etc.).
 func streamWithReconnect(ctx context.Context, provider Provider, request zeroruntime.CompletionRequest, notify reconnectNotifier) (<-chan zeroruntime.StreamEvent, error) {
-	recorder := trace.FromContext(ctx)
 	stream, err := provider.StreamCompletion(ctx, request)
 	if err == nil {
-		if recorder != nil {
-			recorder.Counter(trace.CounterModelRequests, 1)
-		}
 		return stream, nil
 	}
 	for attempt := 1; attempt <= maxStreamReconnects; attempt++ {
 		if !shouldReconnect(ctx, err) {
 			return nil, err
 		}
-		if recorder != nil {
-			recorder.Counter(trace.CounterReconnectCount, 1)
-		}
 		if notify != nil {
 			notify(attempt, maxStreamReconnects)
 		}
@@ -98,9 +90,6 @@ func streamWithReconnect(ctx context.Context, provider Provider, request zerorun
 		}
 		stream, err = provider.StreamCompletion(ctx, request)
 		if err == nil {
-			if recorder != nil {
-				recorder.Counter(trace.CounterModelRequests, 1)
-			}
 			return stream, nil
 		}
 	}

internal/agent/selfcorrect.go

@@ -9,7 +9,6 @@ import (
 
 	"github.com/Gitlawb/zero/internal/lsp"
 	"github.com/Gitlawb/zero/internal/redaction"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/verify"
 )
 
@@ -134,9 +133,7 @@ func (sc *SelfCorrector) inspect(ctx context.Context, changedFiles []string) Cor
 	}
 
 	if sc.cfg.IncludeTests && sc.verifier != nil {
-		verifySpan := trace.FromContext(ctx).Span(trace.SpanVerification)
 		vr, err := sc.verifier.Verify(ctx)
-		verifySpan.End()
 		if err != nil {
 			// "No plan / no tests" is not an error (DetectPlan returns an empty
 			// plan), so a non-nil error means verification could not run at all —

internal/agent/types.go

@@ -7,7 +7,6 @@ import (
 	"github.com/Gitlawb/zero/internal/sandbox"
 	"github.com/Gitlawb/zero/internal/streamjson"
 	"github.com/Gitlawb/zero/internal/tools"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -280,16 +279,6 @@ type Options struct {
 	// request), so every existing caller is unaffected. A returned error is
 	// non-fatal: the run continues on the current model.
 	ModelSwitcher func(ctx context.Context, modelID string) (Provider, error)
-	// Trace, when set, records per-turn timing for the run: the loop stamps
-	// spans (prompt build, generation, tool execution, permission wait,
-	// compaction, provider connect) and counters (model requests, tool calls,
-	// retries, tokens) into it. nil DISABLES tracing entirely — every stamp is
-	// nil-safe and the loop is byte-identical to an untraced run. The caller
-	// owns the recorder: Run stamps into it but does not Finish or emit it.
-	// A fresh Recorder is required per Run — reusing one across runs merges
-	// their spans, counters, and first-event timestamps, and Finish freezes a
-	// recorder so no further stamps take.
-	Trace *trace.Recorder
 	// SelfCorrect, when set, runs a post-edit verify-and-correct cycle after a
 	// mutating tool call: it verifies the changed files (LSP diagnostics + project
 	// tests) and feeds failures back to the model to fix, bounded by an attempt
@@ -307,13 +296,13 @@ type Options struct {
 
 	// RequireCompletionSignal gates run completion for HEADLESS exec. Without it,
 	// any assistant turn that produces text but no tool call is accepted as the
-	// final answer. With it, a no-tool-call turn is NOT treated as "done" while
-	// work clearly remains — pending update_plan items, or a message that ends on a
-	// continuation cue ("…Let me check the config:"). The loop then nudges the
-	// model to continue instead, bounded by maxContinueNudges (and still by
-	// MaxTurns and the run deadline); if the model keeps stalling, the run
-	// finalizes as INCOMPLETE (Result.Incomplete) rather than success. Default
-	// false leaves the loop byte-identical, so the interactive TUI is unaffected.
+	// final answer — except while update_plan still has pending/in_progress items,
+	// when the plan-aware completion gate always applies (interactive and headless).
+	// With RequireCompletionSignal, a no-tool-call turn is also gated on
+	// continuation cues ("…Let me check the config:"). The loop nudges the model to
+	// continue instead, bounded by maxContinueNudges (and still by MaxTurns and the
+	// run deadline); if the model keeps stalling, the run finalizes as INCOMPLETE
+	// (Result.Incomplete) rather than success.
 	RequireCompletionSignal bool
 
 	runPermissions *permissionRunState

internal/cli/app.go

@@ -42,7 +42,6 @@ import (
 	"github.com/Gitlawb/zero/internal/worktrees"
 	"github.com/Gitlawb/zero/internal/zerogit"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
-	"github.com/charmbracelet/x/term"
 )
 
 var version = "dev"
@@ -340,17 +339,7 @@ func runWithDeps(args []string, stdout io.Writer, stderr io.Writer, deps appDeps
 				return 0
 			}
 		}
-		// Pipes and redirects keep the machine-readable contract exactly as it
-		// was: a single "zero <version>" line. Scripts, command substitutions,
-		// and the NPM wrapper smoke check all parse that record, so the banner
-		// is strictly a TTY affordance.
-		if !stdoutIsTerminal(stdout) {
-			if _, err := fmt.Fprintf(stdout, "zero %s\n", version); err != nil {
-				return 1
-			}
-			return 0
-		}
-		if _, err := fmt.Fprintf(stdout, "%s\n\nzero %s\n", tui.Wordmark(), version); err != nil {
+		if _, err := fmt.Fprintf(stdout, "zero %s\n", version); err != nil {
 			return 1
 		}
 		return 0
@@ -1126,17 +1115,6 @@ func closeSpecialistRuntime(stderr io.Writer, runtime *agentToolRuntime) {
 	}
 }
 
-// stdoutIsTerminal reports whether w is an interactive terminal. Tests pass
-// bytes.Buffer writers and pipes/redirects fail term.IsTerminal, so both take
-// the machine-readable path.
-func stdoutIsTerminal(w io.Writer) bool {
-	f, ok := w.(*os.File)
-	if !ok {
-		return false
-	}
-	return term.IsTerminal(f.Fd())
-}
-
 func writeAppError(stderr io.Writer, message string, exitCode int) int {
 	if _, err := fmt.Fprintf(stderr, "[zero] %s\n", message); err != nil {
 		return 1

internal/cli/app_test.go

@@ -41,8 +41,6 @@ func TestRunPrintsVersion(t *testing.T) {
 	if exitCode != 0 {
 		t.Fatalf("expected exit code 0, got %d", exitCode)
 	}
-	// Non-terminal stdout keeps the machine-readable contract: exactly one
-	// "zero <version>" record, no wordmark banner, no ANSI.
 	if got := stdout.String(); got != "zero dev\n" {
 		t.Fatalf("expected version output, got %q", got)
 	}
@@ -51,33 +49,6 @@ func TestRunPrintsVersion(t *testing.T) {
 	}
 }
 
-// TestRunVersionRedirectedFile covers the `zero --version > file` path: stdout
-// is a real *os.File but not a terminal, so the single-line contract must hold.
-func TestRunVersionRedirectedFile(t *testing.T) {
-	stdout, err := os.CreateTemp(t.TempDir(), "version-stdout")
-	if err != nil {
-		t.Fatalf("create temp stdout: %v", err)
-	}
-	defer stdout.Close()
-	var stderr bytes.Buffer
-
-	exitCode := Run([]string{"--version"}, stdout, &stderr)
-
-	if exitCode != 0 {
-		t.Fatalf("expected exit code 0, got %d", exitCode)
-	}
-	content, err := os.ReadFile(stdout.Name())
-	if err != nil {
-		t.Fatalf("read temp stdout: %v", err)
-	}
-	if got := string(content); got != "zero dev\n" {
-		t.Fatalf("expected version output, got %q", got)
-	}
-	if stderr.Len() != 0 {
-		t.Fatalf("expected empty stderr, got %q", stderr.String())
-	}
-}
-
 func TestRunPrintsHelp(t *testing.T) {
 	for _, args := range [][]string{
 		{"--help"},

internal/cli/distribution.go

@@ -118,8 +118,7 @@ func runSkillInfo(args []string, dir string, stdout io.Writer, stderr io.Writer)
 	if name == "" {
 		return writeExecUsageError(stderr, "usage: zero skill info <name> [--json]")
 	}
-	// Resolve across primary + ~/.agents/skills; lock metadata only from primary.
-	info, ok := skills.InfoFromRoots(dir, skills.GlobalRoots(dir), name)
+	info, ok := skills.Info(dir, name)
 	if !ok {
 		return writeAppError(stderr, fmt.Sprintf("skill %q not found", name), exitUsage)
 	}

internal/cli/exec.go

@@ -25,7 +25,6 @@ import (
 	"github.com/Gitlawb/zero/internal/specmode"
 	"github.com/Gitlawb/zero/internal/streamjson"
 	"github.com/Gitlawb/zero/internal/tools"
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/usage"
 	"github.com/Gitlawb/zero/internal/worktrees"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
@@ -125,12 +124,6 @@ type execOptions struct {
 	// additional write roots for this run. Unioned with
 	// config.SandboxConfig.AdditionalWriteRoots at scope construction time.
 	addDirs []string
-	// tracePath, when set, writes a per-turn NDJSON trace (agenteval-compatible)
-	// to the given file path — or to stderr when the value is "-". Falls back to
-	// the ZERO_TRACE env var when the flag is absent. Off by default: a run
-	// without it leaves agent.Options.Trace nil and is byte-identical to before.
-	// The trace is pure observation — enabling it does not change agent behavior.
-	tracePath string
 }
 
 type execUsageError struct {
@@ -429,12 +422,6 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
 	notify.MaybeAddWebhookSink(notifier, os.Getenv, func(format string, args ...any) {
 		fmt.Fprintf(stderr, "[notify] "+format+"\n", args...)
 	})
-	// Spec-draft runs synthesize a prompt offline and never drive a model turn, so
-	// there is no per-turn trace to emit. Reject --trace / ZERO_TRACE up front with
-	// a clear error rather than silently accepting and writing nothing.
-	if options.useSpec && resolveTracePath(options) != "" {
-		return writeExecFormatUsageError(stdout, stderr, options.outputFormat, "--trace / ZERO_TRACE are not supported for spec-draft runs")
-	}
 	if options.useSpec {
 		return runExecSpecDraft(execSpecDraftRun{
 			options:            options,
@@ -487,24 +474,6 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
 	if err != nil {
 		return writeAppError(stderr, "failed to create run id: "+err.Error(), exitCrash)
 	}
-	// Per-turn tracing is opt-in (--trace <path> or ZERO_TRACE=<path>). The
-	// recorder is stamped throughout agent.Run and the providerio seam; the run
-	// itself is byte-identical to an untraced run. We finish the recorder at the
-	// agent.Run boundary (below) so the snapshot captures exactly one turn, then
-	// serialize the snapshot on every exit path via the defer. "-" writes NDJSON
-	// to stderr; otherwise the path is created/truncated. A failure to emit is
-	// logged to stderr, never fatal.
-	tracePath := resolveTracePath(options)
-	var traceRecorder *trace.Recorder
-	var traceSnapshot *trace.TurnTrace
-	if tracePath != "" {
-		traceRecorder = trace.NewRecorder(preparedSession.Session.SessionID, runID, "")
-		defer func() {
-			if err := writeTraceSnapshot(traceSnapshot, tracePath, stderr); err != nil {
-				fmt.Fprintf(stderr, "[zero] failed to write trace: %s\n", err)
-			}
-		}()
-	}
 	writer := execEventWriter{
 		stdout:       stdout,
 		stderr:       stderr,
@@ -587,7 +556,6 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
 		Model:            resolved.Provider.Model,
 		ModelSwitcher:    modelSwitcher,
 		ReasoningEffort:  forwardEffort,
-		Trace:            traceRecorder,
 		Cwd:              workspaceRoot,
 		Images:           images,
 		Registry:         registry,
@@ -657,12 +625,6 @@ func runExec(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) in
 			sessionRecorder.append(sessions.EventUsage, payload)
 		},
 	})
-	// Finish the trace now that the turn is done, so the snapshot captures exactly
-	// agent.Run's work and nothing the post-run cleanup stamps. The deferred
-	// writer serializes the snapshot on every exit path.
-	if traceRecorder != nil {
-		traceSnapshot = traceRecorder.Finish()
-	}
 	notifier.Notify(notify.Completion, notify.DefaultMessage(notify.Completion))
 	if writer.err != nil {
 		return exitCrash
@@ -1275,36 +1237,3 @@ func execNotifyMode(options execOptions, resolved config.ResolvedConfig) string
 	}
 	return resolved.Notify.Mode
 }
-
-// resolveTracePath returns the trace destination for this run: the --trace flag
-// value when set, else the ZERO_TRACE env var, else "" (tracing off). A value of
-// "-" means "write to stderr".
-func resolveTracePath(options execOptions) string {
-	if v := strings.TrimSpace(options.tracePath); v != "" {
-		return v
-	}
-	if v := strings.TrimSpace(os.Getenv("ZERO_TRACE")); v != "" {
-		return v
-	}
-	return ""
-}
-
-// writeTraceSnapshot writes an already-finished trace snapshot to dest ("-" =>
-// stderr, otherwise a file path created/truncated). A nil snapshot is a no-op
-// (tracing off, or the run exited before agent.Run produced a turn). The trace
-// is best-effort: a write error is returned to the caller, which logs it but
-// never fails the run.
-func writeTraceSnapshot(snapshot *trace.TurnTrace, dest string, stderr io.Writer) error {
-	if snapshot == nil || dest == "" {
-		return nil
-	}
-	if strings.TrimSpace(dest) == "-" {
-		return trace.WriteNDJSON(stderr, snapshot)
-	}
-	file, err := os.Create(dest)
-	if err != nil {
-		return err
-	}
-	defer file.Close()
-	return trace.WriteNDJSON(file, snapshot)
-}

internal/cli/exec_parse.go

@@ -32,15 +32,6 @@ func parseExecArgs(args []string) (execOptions, bool, error) {
 			options.noNotify = true
 		case arg == "--no-completion-gate":
 			options.noCompletionGate = true
-		case arg == "--trace":
-			value, next, err := nextFlagValue(args, index, arg)
-			if err != nil {
-				return options, false, err
-			}
-			options.tracePath = value
-			index = next
-		case strings.HasPrefix(arg, "--trace="):
-			options.tracePath = strings.TrimSpace(strings.TrimPrefix(arg, "--trace="))
 		case arg == "--notify":
 			value, next, err := nextFlagValue(args, index, arg)
 			if err != nil {

internal/cli/plugin_activate.go

@@ -15,11 +15,11 @@ import (
 
 // pluginActivation holds what plugin activation contributed to a bootstrap so the
 // later dispatcher + skill wiring can consume it: the plugin hook definitions and
-// the plugin skill search roots. The zero value (no plugins) still overlays the
-// multi-root skill tool (primary + ~/.agents/skills) so discovery is identical
-// with or without plugins. The embedded trustSkip reports whether the project
-// plugin layer was dropped for an untrusted workspace, so the caller can fold it
-// into the single combined trust notice.
+// the plugin skill search roots. The zero value (no plugins) is inert — the
+// dispatcher gets no extra hooks and the skill tool keeps only the default dir.
+// The embedded trustSkip reports whether the project plugin layer was dropped for
+// an untrusted workspace, so the caller can fold it into the single combined
+// trust notice.
 type pluginActivation struct {
 	hooks      []hooks.Definition
 	skillRoots []string
@@ -52,9 +52,6 @@ func activatePlugins(workspaceRoot string, registry *tools.Registry, deps appDep
 	loaded, err := deps.loadPlugins(plugins.LoadOptions{Cwd: workspaceRoot, ExcludeProject: excludeProject})
 	if err != nil {
 		writePluginActivationWarning(stderr, "failed to load plugins: "+err.Error())
-		// Still overlay multi-root discovery (primary + agents) so a plugin load
-		// failure cannot hide shared skills.
-		registry.Register(plugins.NewSkillTool(deps.skillsDir(), nil))
 		return pluginActivation{trustSkip: skip}
 	}
 
@@ -70,11 +67,14 @@ func activatePlugins(workspaceRoot string, registry *tools.Registry, deps appDep
 		writePluginActivationWarning(stderr, warning)
 	}
 
-	// Always overlay the multi-root skill tool so discovery is identical with or
-	// without plugins: primary DefaultDir + optional ~/.agents/skills + plugin
-	// roots (earlier wins). With empty SkillRoots and no agents dir this is
-	// byte-equivalent to the core single-dir skill surface.
-	registry.Register(plugins.NewSkillTool(deps.skillsDir(), result.SkillRoots))
+	// Re-register the skill tool to also resolve plugin-declared skills. The core
+	// skill tool only reads the default skills dir; the plugin-aware replacement
+	// merges the default dir with the plugin skill roots (default dir wins a name
+	// clash), so plugin skills appear in the agent's skill list. With no plugin
+	// skill roots this is byte-equivalent to the default skills surface.
+	if len(result.SkillRoots) > 0 {
+		registry.Register(plugins.NewSkillTool(deps.skillsDir(), result.SkillRoots))
+	}
 
 	return pluginActivation{hooks: result.Hooks, skillRoots: result.SkillRoots, trustSkip: skip}
 }
@@ -91,8 +91,8 @@ func projectPluginsDirExists(workspaceRoot string) bool {
 }
 
 // skillInfos resolves the reusable skills the model can load via the skill tool —
-// primary dir + optional ~/.agents/skills + plugin skill roots, the same set the
-// multi-root skill tool resolves against — as plain data for the agent's system
+// the default skills dir merged with any plugin skill roots, the same set the
+// plugin-aware skill tool resolves against — as plain data for the agent's system
 // prompt. It returns nil when no skills are installed, so a skill-less run leaves
 // the prompt byte-identical.
 func (a pluginActivation) skillInfos(defaultDir string) []agent.SkillInfo {

internal/cli/plugin_activate_test.go

@@ -69,11 +69,6 @@ func TestActivatePluginsRegistersToolAndCollectsHooks(t *testing.T) {
 }
 
 func TestActivatePluginsRegistersPluginSkillTool(t *testing.T) {
-	// Isolate host ~/.agents/skills so it cannot interfere with this test.
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-
 	pluginDir := t.TempDir()
 	skillDir := filepath.Join(pluginDir, "skills", "demo-skill")
 	if err := os.MkdirAll(skillDir, 0o755); err != nil {
@@ -113,42 +108,6 @@ func TestActivatePluginsRegistersPluginSkillTool(t *testing.T) {
 	}
 }
 
-func TestActivatePluginsAlwaysRegistersMultiRootSkillTool(t *testing.T) {
-	// Even with zero plugins / zero plugin skill roots, activation must overlay
-	// the multi-root skill tool so ~/.agents/skills is loadable on bare runs.
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	skillDir := filepath.Join(agents, "agents-skill")
-	if err := os.MkdirAll(skillDir, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte("---\nname: agents-skill\n---\nagents body"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-
-	registry := tools.NewRegistry()
-	registry.Register(tools.NewSkillTool(t.TempDir())) // core single-dir tool
-	var stderr bytes.Buffer
-	workspace := t.TempDir()
-	activation := activatePlugins(workspace, registry, fakePluginDeps(t, nil), &stderr, workspace)
-	if len(activation.skillRoots) != 0 {
-		t.Fatalf("expected no plugin skill roots, got %#v", activation.skillRoots)
-	}
-	skillTool, ok := registry.Get("skill")
-	if !ok {
-		t.Fatal("skill tool missing after activation")
-	}
-	res := skillTool.Run(context.Background(), map[string]any{"name": "agents-skill"})
-	if res.Status != tools.StatusOK || !bytes.Contains([]byte(res.Output), []byte("agents body")) {
-		t.Fatalf("agents skill must load with zero plugin roots: %q (%s)", res.Status, res.Output)
-	}
-}
-
 func TestActivatePluginsSurfacesLoadDiagnostics(t *testing.T) {
 	// Load succeeds (no top-level error) but reports a per-plugin diagnostic for a
 	// plugin it had to skip. activatePlugins must forward that diagnostic to stderr

internal/cli/provider_catalog_onboarding_test.go

@@ -17,7 +17,6 @@ func TestProviderCatalogRuntimeProvidersShowOnboardingSetupHints(t *testing.T) {
 		defaultModel string
 	}{
 		{id: "openai", name: "OpenAI", transport: "openai", defaultModel: "gpt-4.1"},
-		{id: "aimlapi", name: "AI/ML API", transport: "openai-compatible", defaultModel: "openai/gpt-5-chat"},
 		{id: "groq", name: "Groq", transport: "openai-compatible", defaultModel: "llama-3.3-70b-versatile"},
 		{id: "longcat", name: "LongCat", transport: "openai-compatible", defaultModel: "LongCat-2.0"},
 		{id: "ollama-cloud", name: "Ollama Cloud", transport: "openai-compatible", defaultModel: "qwen3-coder:480b"},
@@ -48,21 +47,6 @@ func TestProviderCatalogRuntimeProvidersShowOnboardingSetupHints(t *testing.T) {
 	}
 }
 
-func TestProviderCatalogAIMLAPIRequiresAPIKey(t *testing.T) {
-	output := runProviderCatalogOnboarding(t)
-	block := providerCatalogOnboardingBlock(t, output, "aimlapi")
-
-	for _, want := range []string{
-		"requiresAuth=true",
-		"authEnvVars=AIMLAPI_API_KEY",
-		"setup: zero providers setup aimlapi --set-active",
-	} {
-		if !strings.Contains(block, want) {
-			t.Fatalf("expected AIMLAPI catalog block to contain %q, got:\n%s", want, block)
-		}
-	}
-}
-
 func TestProviderCatalogLongCatRequiresAPIKey(t *testing.T) {
 	output := runProviderCatalogOnboarding(t)
 	block := providerCatalogOnboardingBlock(t, output, "longcat")

internal/cli/serve.go

@@ -4,8 +4,6 @@ import (
 	"context"
 	"fmt"
 	"io"
-	"os"
-	"path/filepath"
 	"strings"
 
 	"github.com/Gitlawb/zero/internal/mcp"
@@ -16,21 +14,9 @@ import (
 type serveOptions struct {
 	mcp              bool
 	cwd              string
-	addDirs          []string
 	allowUnsafeTools bool
 }
 
-// serveRootsScope is a PathScope used only for MCP serve: the workspace root
-// plus optional --add-dir roots, without the sandbox temp write roots that
-// would otherwise pollute resources/list.
-type serveRootsScope []string
-
-func (s serveRootsScope) Roots() []string {
-	roots := make([]string, len(s))
-	copy(roots, s)
-	return roots
-}
-
 func runServe(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int {
 	options, help, err := parseServeArgs(args)
 	if err != nil {
@@ -50,11 +36,7 @@ func runServe(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) i
 	if err != nil {
 		return writeExecUsageError(stderr, err.Error())
 	}
-	scope, err := buildServeScope(workspaceRoot, options.addDirs)
-	if err != nil {
-		return writeExecUsageError(stderr, err.Error())
-	}
-	registry := newServeRegistry(workspaceRoot, scope, options.allowUnsafeTools)
+	registry := newServeRegistry(workspaceRoot, options.allowUnsafeTools)
 	if options.allowUnsafeTools {
 		if _, err := fmt.Fprintln(stderr, "[zero] Unsafe MCP server tools enabled because --allow-unsafe-tools was passed."); err != nil {
 			return exitCrash
@@ -65,8 +47,6 @@ func runServe(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) i
 		Name:              "zero",
 		Version:           version,
 		PermissionGranted: options.allowUnsafeTools,
-		WorkspaceRoot:     workspaceRoot,
-		Scope:             scope,
 	})
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
@@ -74,11 +54,11 @@ func runServe(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) i
 	return exitSuccess
 }
 
-func newServeRegistry(workspaceRoot string, scope tools.PathScope, allowUnsafeTools bool) *tools.Registry {
+func newServeRegistry(workspaceRoot string, allowUnsafeTools bool) *tools.Registry {
 	registry := tools.NewRegistry()
-	toolset := tools.CoreReadOnlyToolsScoped(workspaceRoot, scope)
+	toolset := tools.CoreReadOnlyTools(workspaceRoot)
 	if allowUnsafeTools {
-		toolset = tools.CoreToolsScoped(workspaceRoot, scope)
+		toolset = tools.CoreTools(workspaceRoot)
 	}
 	for _, tool := range toolset {
 		registry.Register(tool)
@@ -86,58 +66,6 @@ func newServeRegistry(workspaceRoot string, scope tools.PathScope, allowUnsafeTo
 	return registry
 }
 
-// buildServeScope returns nil when there are no extra roots (workspace-only),
-// otherwise a PathScope listing the workspace first followed by each --add-dir.
-func buildServeScope(workspaceRoot string, addDirs []string) (tools.PathScope, error) {
-	if len(addDirs) == 0 {
-		return nil, nil
-	}
-	roots := make([]string, 0, 1+len(addDirs))
-	workspaceRoot = filepath.Clean(workspaceRoot)
-	if abs, err := filepath.Abs(workspaceRoot); err == nil {
-		workspaceRoot = abs
-	}
-
-	// Keep lexical absolute paths in roots so path checks stay aligned with the
-	// un-evaluated WorkspaceRoot used when Scope is nil. Symlink-resolved forms
-	// are only keys for duplicate detection.
-	workspaceResolved := workspaceRoot
-	if resolved, err := filepath.EvalSymlinks(workspaceRoot); err == nil {
-		workspaceResolved = resolved
-	}
-	roots = append(roots, workspaceRoot)
-
-	seen := map[string]struct{}{workspaceResolved: {}}
-	for _, dir := range addDirs {
-		trimmed := strings.TrimSpace(dir)
-		if trimmed == "" {
-			return nil, execUsageError{"--add-dir requires a path"}
-		}
-		absolute, err := filepath.Abs(trimmed)
-		if err != nil {
-			return nil, execUsageError{fmt.Sprintf("--add-dir %q: %v", trimmed, err)}
-		}
-		info, err := os.Stat(absolute)
-		if err != nil {
-			return nil, execUsageError{fmt.Sprintf("--add-dir %q: %v", trimmed, err)}
-		}
-		if !info.IsDir() {
-			return nil, execUsageError{fmt.Sprintf("--add-dir %q is not a directory", trimmed)}
-		}
-
-		resolved := absolute
-		if r, err := filepath.EvalSymlinks(absolute); err == nil {
-			resolved = r
-		}
-		if _, ok := seen[resolved]; ok {
-			continue
-		}
-		seen[resolved] = struct{}{}
-		roots = append(roots, absolute)
-	}
-	return serveRootsScope(roots), nil
-}
-
 func parseServeArgs(args []string) (serveOptions, bool, error) {
 	options := serveOptions{}
 	for index := 0; index < len(args); index++ {
@@ -157,18 +85,6 @@ func parseServeArgs(args []string) (serveOptions, bool, error) {
 			options.cwd = args[index]
 		case strings.HasPrefix(arg, "--cwd="):
 			options.cwd = strings.TrimPrefix(arg, "--cwd=")
-		case arg == "--add-dir":
-			index++
-			if index >= len(args) {
-				return options, false, execUsageError{"--add-dir requires a path"}
-			}
-			options.addDirs = append(options.addDirs, args[index])
-		case strings.HasPrefix(arg, "--add-dir="):
-			value := strings.TrimPrefix(arg, "--add-dir=")
-			if strings.TrimSpace(value) == "" {
-				return options, false, execUsageError{"--add-dir requires a path"}
-			}
-			options.addDirs = append(options.addDirs, value)
 		default:
 			return options, false, execUsageError{fmt.Sprintf("unknown serve flag %q", arg)}
 		}
@@ -184,8 +100,7 @@ Starts Zero as an MCP stdio server.
 
 Flags:
       --mcp                   Run the MCP stdio server
-  -C, --cwd <path>            Set the workspace directory (resources + tools)
-      --add-dir <path>        Extra resource/tool root (repeatable)
+  -C, --cwd <path>            Set the workspace directory
       --allow-unsafe-tools    Expose write and shell tools to the MCP host
   -h, --help                  Show this help
 `)

internal/cli/serve_test.go

@@ -4,8 +4,6 @@ import (
 	"bytes"
 	"encoding/json"
 	"fmt"
-	"os"
-	"path/filepath"
 	"strings"
 	"testing"
 )
@@ -82,120 +80,6 @@ func TestRunServeRequiresMCPMode(t *testing.T) {
 	}
 }
 
-func TestRunServeMCPWiresWorkspaceRootAndAddDirIntoResources(t *testing.T) {
-	workspace := t.TempDir()
-	extra := t.TempDir()
-	if err := os.WriteFile(filepath.Join(workspace, "workspace.txt"), []byte("ws\n"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(extra, "extra.txt"), []byte("ex\n"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-
-	var stdout bytes.Buffer
-	var stderr bytes.Buffer
-	exitCode := runWithDeps(
-		[]string{"serve", "--mcp", "-C", workspace, "--add-dir", extra},
-		&stdout,
-		&stderr,
-		appDeps{stdin: bytes.NewReader(serveMCPResourcesInput(t))},
-	)
-	if exitCode != exitSuccess {
-		t.Fatalf("exit=%d stderr=%q", exitCode, stderr.String())
-	}
-	output := stdout.String()
-	if !strings.Contains(output, "workspace.txt") {
-		t.Fatalf("expected workspace resource, got %q", output)
-	}
-	if !strings.Contains(output, "extra.txt") {
-		t.Fatalf("expected --add-dir resource, got %q", output)
-	}
-}
-
-func TestRunServeMCPRejectsMissingAddDir(t *testing.T) {
-	workspace := t.TempDir()
-	missing := filepath.Join(workspace, "does-not-exist")
-	var stdout bytes.Buffer
-	var stderr bytes.Buffer
-	exitCode := runWithDeps(
-		[]string{"serve", "--mcp", "-C", workspace, "--add-dir", missing},
-		&stdout,
-		&stderr,
-		appDeps{stdin: bytes.NewReader(nil)},
-	)
-	if exitCode != exitUsage {
-		t.Fatalf("exit=%d want %d stderr=%q", exitCode, exitUsage, stderr.String())
-	}
-	if !strings.Contains(stderr.String(), "--add-dir") {
-		t.Fatalf("expected --add-dir error, got %q", stderr.String())
-	}
-}
-
-func TestParseServeArgsCollectsAddDirs(t *testing.T) {
-	options, help, err := parseServeArgs([]string{"--mcp", "--add-dir", "/one", "--add-dir=/two", "-C", "/ws"})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if help {
-		t.Fatal("unexpected help")
-	}
-	if !options.mcp || options.cwd != "/ws" {
-		t.Fatalf("options=%#v", options)
-	}
-	if len(options.addDirs) != 2 || options.addDirs[0] != "/one" || options.addDirs[1] != "/two" {
-		t.Fatalf("addDirs=%v", options.addDirs)
-	}
-}
-
-func TestBuildServeScopeNilWithoutExtras(t *testing.T) {
-	scope, err := buildServeScope(t.TempDir(), nil)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if scope != nil {
-		t.Fatalf("expected nil scope, got %#v", scope)
-	}
-}
-
-func TestBuildServeScopeKeepsLexicalPaths(t *testing.T) {
-	base := t.TempDir()
-	realWorkspace := filepath.Join(base, "real-workspace")
-	realExtra := filepath.Join(base, "real-extra")
-	for _, dir := range []string{realWorkspace, realExtra} {
-		if err := os.MkdirAll(dir, 0o700); err != nil {
-			t.Fatal(err)
-		}
-	}
-	linkWorkspace := filepath.Join(base, "link-workspace")
-	linkExtra := filepath.Join(base, "link-extra")
-	if err := os.Symlink(realWorkspace, linkWorkspace); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Symlink(realExtra, linkExtra); err != nil {
-		t.Fatal(err)
-	}
-
-	scope, err := buildServeScope(linkWorkspace, []string{linkExtra})
-	if err != nil {
-		t.Fatal(err)
-	}
-	if scope == nil {
-		t.Fatal("expected non-nil scope")
-	}
-	roots := scope.Roots()
-	wantWorkspace, err := filepath.Abs(linkWorkspace)
-	if err != nil {
-		t.Fatal(err)
-	}
-	wantExtra, err := filepath.Abs(linkExtra)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if len(roots) != 2 || roots[0] != wantWorkspace || roots[1] != wantExtra {
-		t.Fatalf("roots=%v want [%q %q]", roots, wantWorkspace, wantExtra)
-	}
-}
-
 func serveMCPInput(t *testing.T) []byte {
 	t.Helper()
 
@@ -220,30 +104,6 @@ func serveMCPInput(t *testing.T) []byte {
 	return input.Bytes()
 }
 
-func serveMCPResourcesInput(t *testing.T) []byte {
-	t.Helper()
-
-	var input bytes.Buffer
-	writeServeMCPMessage(t, &input, map[string]any{
-		"jsonrpc": "2.0",
-		"id":      1,
-		"method":  "initialize",
-		"params":  map[string]any{},
-	})
-	writeServeMCPMessage(t, &input, map[string]any{
-		"jsonrpc": "2.0",
-		"method":  "notifications/initialized",
-		"params":  map[string]any{},
-	})
-	writeServeMCPMessage(t, &input, map[string]any{
-		"jsonrpc": "2.0",
-		"id":      2,
-		"method":  "resources/list",
-		"params":  map[string]any{},
-	})
-	return input.Bytes()
-}
-
 func writeServeMCPMessage(t *testing.T, buffer *bytes.Buffer, message map[string]any) {
 	t.Helper()
 	body, err := json.Marshal(message)

internal/cli/skills.go

@@ -59,21 +59,25 @@ func runSkills(args []string, stdout io.Writer, stderr io.Writer, deps appDeps)
 }
 
 func runSkillsList(dir string, options skillListOptions, stdout io.Writer, stderr io.Writer) int {
-	// Management CLI lists global roots only (primary + ~/.agents/skills); plugin
-	// skills stay out of `zero skills list` and surface via the agent skill tool.
-	roots := skills.GlobalRoots(dir)
-	discovered, dups, err := skills.ListFromRoots(roots)
+	discovered, err := skills.List(dir)
 	if err != nil {
 		return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
 	}
-	// Surface name collisions that ListFromRoots silently resolved (earlier root
-	// wins), so a shadowed same-named skill is reported instead of just
-	// disappearing. Warnings go to stderr, keeping stdout (including --json) clean.
-	for _, dup := range dups {
-		fmt.Fprintf(stderr, "warning: duplicate skill %q: using %s, ignoring %s\n",
-			redaction.RedactString(dup.Name, redaction.Options{}),
-			redaction.RedactString(dup.Winner, redaction.Options{}),
-			redaction.RedactString(dup.Loser, redaction.Options{}))
+	// Surface name collisions that List silently resolved (first directory wins),
+	// so a shadowed same-named skill is reported instead of just disappearing.
+	// Warnings go to stderr, keeping stdout (including --json) clean.
+	if dups, derr := skills.Duplicates(dir); derr == nil {
+		for _, dup := range dups {
+			fmt.Fprintf(stderr, "warning: duplicate skill %q: using %s, ignoring %s\n",
+				redaction.RedactString(dup.Name, redaction.Options{}),
+				redaction.RedactString(dup.Winner, redaction.Options{}),
+				redaction.RedactString(dup.Loser, redaction.Options{}))
+		}
+	} else {
+		// Don't silently swallow a scan failure: "no warnings" would then be
+		// ambiguous (no duplicates vs. detection broke). Surface it on stderr.
+		fmt.Fprintf(stderr, "warning: could not check for duplicate skills: %s\n",
+			redaction.ErrorMessage(derr, redaction.Options{}))
 	}
 	if options.json {
 		payload := struct {
@@ -84,16 +88,16 @@ func runSkillsList(dir string, options skillListOptions, stdout io.Writer, stder
 		}
 		return exitSuccess
 	}
-	output := redaction.RedactString(formatSkillList(discovered), redaction.Options{})
+	output := redaction.RedactString(formatSkillList(discovered, dir), redaction.Options{})
 	if _, err := fmt.Fprintln(stdout, output); err != nil {
 		return exitCrash
 	}
 	return exitSuccess
 }
 
-func formatSkillList(discovered []skills.Skill) string {
+func formatSkillList(discovered []skills.Skill, dir string) string {
 	if len(discovered) == 0 {
-		return "No skills found."
+		return fmt.Sprintf("No Zero skills found in %s.", dir)
 	}
 	lines := []string{"Zero Skills:"}
 	for _, skill := range discovered {
@@ -127,13 +131,10 @@ func writeSkillsHelp(w io.Writer) error {
   zero skills <command>
 
 Commands:
-  list                 List discovered skills (Zero dir + ~/.agents/skills)
-  add <git-url|path>   Install a skill into the Zero skills dir (checksum-pinned in skills.lock)
+  list                 List discovered Zero skills
+  add <git-url|path>   Install a skill (checksum-pinned in skills.lock)
   info <name>          Show a skill's frontmatter, source, and pinned hash
-  remove <name>        Remove an installed skill and its lockfile entry from the Zero skills dir
-
-list and info also search ~/.agents/skills when present (read-only).
-add and remove always target the Zero-specific skills directory.
+  remove <name>        Remove an installed skill and its lockfile entry
 `)
 	return err
 }

internal/cli/skills_test.go

@@ -23,15 +23,7 @@ func writeSkillFixture(t *testing.T, dir string, name string, content string) {
 	}
 }
 
-func isolateCLIAgentsHome(t *testing.T) {
-	t.Helper()
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-}
-
 func TestRunSkillsListText(t *testing.T) {
-	isolateCLIAgentsHome(t)
 	dir := t.TempDir()
 	writeSkillFixture(t, dir, "confirmation-policy", "---\nname: confirmation-policy\ndescription: Ask before risky actions.\n---\nbody")
 
@@ -51,7 +43,6 @@ func TestRunSkillsListText(t *testing.T) {
 }
 
 func TestRunSkillsListWarnsOnDuplicateNames(t *testing.T) {
-	isolateCLIAgentsHome(t)
 	dir := t.TempDir()
 	// Two directories declare the same frontmatter name; List keeps one and the
 	// other is shadowed. The command must warn instead of silently dropping it.
@@ -71,7 +62,6 @@ func TestRunSkillsListWarnsOnDuplicateNames(t *testing.T) {
 }
 
 func TestRunSkillsDefaultsToList(t *testing.T) {
-	isolateCLIAgentsHome(t)
 	dir := t.TempDir()
 	writeSkillFixture(t, dir, "demo", "body")
 
@@ -88,7 +78,6 @@ func TestRunSkillsDefaultsToList(t *testing.T) {
 }
 
 func TestRunSkillsListJSON(t *testing.T) {
-	isolateCLIAgentsHome(t)
 	dir := t.TempDir()
 	writeSkillFixture(t, dir, "demo", "---\nname: demo\ndescription: a demo\n---\nbody")
 
@@ -122,7 +111,6 @@ func TestRunSkillsListJSON(t *testing.T) {
 }
 
 func TestRunSkillsEmptyDir(t *testing.T) {
-	isolateCLIAgentsHome(t)
 	var stdout, stderr bytes.Buffer
 	exit := runWithDeps([]string{"skills", "list"}, &stdout, &stderr, appDeps{
 		skillsDir: func() string { return filepath.Join(t.TempDir(), "missing") },
@@ -130,89 +118,11 @@ func TestRunSkillsEmptyDir(t *testing.T) {
 	if exit != 0 {
 		t.Fatalf("exit = %d, stderr = %s", exit, stderr.String())
 	}
-	if !strings.Contains(strings.ToLower(stdout.String()), "no skills found") {
+	if !strings.Contains(strings.ToLower(stdout.String()), "no") {
 		t.Fatalf("expected a no-skills message, got:\n%s", stdout.String())
 	}
 }
 
-func TestRunSkillsListIncludesAgentsOnlySkill(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	writeSkillFixture(t, agents, "agents-only", "---\nname: agents-only\ndescription: Shared multi-agent skill.\n---\nbody")
-
-	primary := t.TempDir()
-	var stdout, stderr bytes.Buffer
-	exit := runWithDeps([]string{"skills", "list"}, &stdout, &stderr, appDeps{
-		skillsDir: func() string { return primary },
-	})
-	if exit != 0 {
-		t.Fatalf("exit = %d, stderr = %s", exit, stderr.String())
-	}
-	out := stdout.String()
-	if !strings.Contains(out, "agents-only") {
-		t.Fatalf("list should include agents-only skill:\n%s", out)
-	}
-	if !strings.Contains(out, agents) {
-		t.Fatalf("list should show agents path:\n%s", out)
-	}
-}
-
-func TestRunSkillsListPrimaryShadowsAgents(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	primary := t.TempDir()
-	writeSkillFixture(t, primary, "shared", "---\nname: shared\ndescription: From primary.\n---\nbody")
-	writeSkillFixture(t, agents, "shared", "---\nname: shared\ndescription: From agents.\n---\nbody")
-
-	var stdout, stderr bytes.Buffer
-	exit := runWithDeps([]string{"skills", "list"}, &stdout, &stderr, appDeps{
-		skillsDir: func() string { return primary },
-	})
-	if exit != 0 {
-		t.Fatalf("exit = %d, stderr = %s", exit, stderr.String())
-	}
-	if !strings.Contains(stdout.String(), "From primary") {
-		t.Fatalf("primary should win list description:\n%s", stdout.String())
-	}
-	if !strings.Contains(stdout.String(), primary) {
-		t.Fatalf("primary path should be shown:\n%s", stdout.String())
-	}
-	if !strings.Contains(stderr.String(), `duplicate skill "shared"`) {
-		t.Fatalf("expected cross-root duplicate warning, got: %q", stderr.String())
-	}
-}
-
-func TestRunSkillInfoResolvesAgentsOnly(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	writeSkillFixture(t, agents, "agents-info", "---\nname: agents-info\ndescription: Agents info.\n---\nbody")
-
-	primary := t.TempDir()
-	var stdout, stderr bytes.Buffer
-	exit := runWithDeps([]string{"skill", "info", "agents-info"}, &stdout, &stderr, appDeps{
-		skillsDir: func() string { return primary },
-	})
-	if exit != 0 {
-		t.Fatalf("exit = %d, stderr = %s", exit, stderr.String())
-	}
-	out := stdout.String()
-	if !strings.Contains(out, "agents-info") || !strings.Contains(out, "Agents info.") {
-		t.Fatalf("info missing agents skill:\n%s", out)
-	}
-	if strings.Contains(out, "source:") || strings.Contains(out, "hash:") {
-		t.Fatalf("agents-only info must not invent lock metadata:\n%s", out)
-	}
-	if !strings.Contains(out, agents) {
-		t.Fatalf("info should show agents path:\n%s", out)
-	}
-}
-
 func TestRunSkillsDoesNotLaunchTUI(t *testing.T) {
 	var stdout, stderr bytes.Buffer
 	launchCalled := false

internal/perfbench/MANIFEST.md

@@ -1,71 +0,0 @@
-# Turn benchmark manifest
-
-The baseline manifest (`manifests/baseline.json`) is the per-turn benchmark's
-program keystone: it defines the tasks the harness runs, the workspace each
-starts in, and — critically — what "pass" means for each task. Because `make
-baseline` is re-run on every perf change, the manifest's pass/fail contract
-matters for months, so the contract is written down here.
-
-## Task count
-
-48 tasks across seven classes:
-
-| class     | count | oracle                      | tier        |
-|-----------|-------|-----------------------------|-------------|
-| nav       | 10    | none                        | latency     |
-| edit      | 10    | substring grep              | correctness |
-| fix       | 8     | scoped `go test -run <name>` | correctness |
-| refactor  | 6     | `go build ./...`            | build       |
-| longproc  | 4     | none                        | latency     |
-| longctx   | 4     | none                        | latency     |
-| parallel  | 6     | none                        | latency     |
-| **total** | **48**|                             |             |
-
-## Oracle tiers
-
-Pass/fail is reported per tier so the report cannot be misread as a blanket
-correctness verdict. The tier is decided per task from oracle presence and the
-manifest's `buildOnlyClasses` list:
-
-- **Correctness** (18 tasks: 10 edit + 8 fix) — a positive oracle (substring
-  grep that the requested change landed, or a scoped `go test -run` that the
-  bug fix passes). This is the only pass rate that can move with model quality:
-  `tasksVerified` / `tasksPassed` / `correctnessPassRate`.
-- **Build-only** (6 tasks: refactor) — `go build ./...` proves the edit
-  compiles, not that the refactor achieved its goal (a no-op refactor passes).
-  The manifest declares `buildOnlyClasses: ["refactor"]`. Reported as
-  `buildCheckedTasks` / `buildPassedTasks` / `buildPassRate`, never in
-  `correctnessPassRate`.
-- **Latency-only** (24 tasks: nav, longproc, longctx, parallel) — no
-  `verificationCommand`. An exit 0 only proves the turn ran, not that the answer
-  was right. They contribute to latency and span attribution only and are
-  counted in `latencyOnlyTasks`, never in any pass rate.
-
-A task's tier is driven by oracle **presence** first: a task with no
-`verificationCommand` is latency-only even if its class is listed in
-`buildOnlyClasses`, so a missing oracle can never silently pass on exit 0.
-
-## Known limitations (deferred)
-
-These are accepted for the Phase 0 baseline and tracked in a follow-up issue;
-they do not block the baseline because the tier split keeps the report honest:
-
-- The edit grep oracles are substring checks. A rename oracle asserts the new
-  name is present AND the old name is gone (compound `bash -c`), but an
-  add-field oracle (`grep -R Label .`) only proves the string appears, not that
-  it landed on the right struct. Strengthening these to `go vet`/`go build` +
-  structural assertions is follow-up work.
-- The refactor build oracle is non-positive (a no-op refactor passes). A
-  structural verifier that proves the refactor happened is follow-up work;
-  until then refactor is `buildPassRate`, not `correctnessPassRate`.
-- The 24 read-only tasks have no oracle. Adding deterministic oracles for nav
-  (diff against an expected answer) and documenting longproc/longctx/parallel
-  as permanently latency-only is follow-up work.
-
-## Fixtures
-
-Each task's `workspaceFixture` points at a small self-contained workspace under
-`testdata/` so the suite runs offline and repeatably. Mutating tasks (edit,
-fix, refactor) run against a per-invocation **copy** of their fixture, so the
-checked-in fixtures stay clean and one task's edits can't bleed into the next
-iteration or a later task.

internal/perfbench/manifests/baseline.json

@@ -1,62 +0,0 @@
-{
-  "id": "zero-baseline-turn",
-  "name": "Zero per-turn baseline (Phase 0)",
-  "description": "Latency-first baseline: measures where a turn spends wall time, with pass/fail reported per oracle tier so it cannot be misread as a blanket correctness verdict. Read-only classes (nav, longproc, longctx, parallel) carry no verification oracle — a zero-exit only proves the turn ran, so they count as latency-only and never in any pass rate. Correctness classes (edit, fix) carry a positive oracle (substring grep or a scoped `go test`) and are the only tasks in correctnessPassRate. Build-only classes (refactor) carry `go build ./...`, which proves the edit compiles but not that the refactor achieved its goal, so they are reported as buildPassRate and excluded from correctnessPassRate.",
-  "buildOnlyClasses": ["refactor"],
-  "tasks": [
-    {"id": "nav-01", "name": "list repo files", "class": "nav", "prompt": "List the files in this repository and report their count. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-02", "name": "find a function", "class": "nav", "prompt": "Find the function named 'greet' and report the file and line it is defined on. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-03", "name": "summarize module", "class": "nav", "prompt": "Read main.go and summarize what this program does in two sentences. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-04", "name": "count tests", "class": "nav", "prompt": "Count the test functions (functions whose names start with Test) in this repository and report the number. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-05", "name": "locate a string", "class": "nav", "prompt": "Find every occurrence of the string 'TODO' in the repository and report the files. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-06", "name": "describe package", "class": "nav", "prompt": "Describe the exported API of this package by reading the source. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-07", "name": "trace a call", "class": "nav", "prompt": "Trace which function calls which other function starting from main. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-08", "name": "find imports", "class": "nav", "prompt": "List every third-party import used by this repository. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-09", "name": "report config", "class": "nav", "prompt": "Read config.json and report the keys it contains. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-    {"id": "nav-10", "name": "doc lookup", "class": "nav", "prompt": "Find the doc comment on the greet function and quote it. Do not modify anything.", "workspaceFixture": "../testdata/nav"},
-
-    {"id": "edit-01", "name": "rename a constant", "class": "edit", "prompt": "Rename the constant MaxRetries to RetryLimit everywhere it appears.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "grep -R RetryLimit . && ! grep -R MaxRetries ."]},
-    {"id": "edit-02", "name": "fix a typo", "class": "edit", "prompt": "Fix the typo in the comment above the greet function ('receieves' should be 'receives').", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "! grep -R receieves ."]},
-    {"id": "edit-03", "name": "add a field", "class": "edit", "prompt": "Add a string field named Label to the Config struct in main.go.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["grep", "-R", "Label", "."]},
-    {"id": "edit-04", "name": "bump a version", "class": "edit", "prompt": "Change the version string in version.go from 1.0.0 to 1.1.0.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "grep -R 1.1.0 . && ! grep -R 1.0.0 ."]},
-    {"id": "edit-05", "name": "remove a log line", "class": "edit", "prompt": "Remove the line that prints 'debug: starting'.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "! grep -R 'debug: starting' ."]},
-    {"id": "edit-06", "name": "add a license header", "class": "edit", "prompt": "Add a comment line '// SPDX-License-Identifier: MIT' at the top of main.go.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["grep", "-R", "SPDX-License-Identifier", "."]},
-    {"id": "edit-07", "name": "change a default", "class": "edit", "prompt": "Change the default value of the Port constant from 8080 to 9090.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "grep -R 9090 . && ! grep -R 8080 ."]},
-    {"id": "edit-08", "name": "add a getter", "class": "edit", "prompt": "Add a method func (c *Config) GetLabel() string that returns c.Label.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["bash", "-c", "grep -R 'func (c *Config) GetLabel' ."]},
-    {"id": "edit-09", "name": "wrap an error", "class": "edit", "prompt": "In the load function, wrap the returned error with fmt.Errorf so it includes 'load failed'.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["grep", "-R", "load failed", "."]},
-    {"id": "edit-10", "name": "update a message", "class": "edit", "prompt": "Change the printed greeting from 'hello' to 'hello, world'.", "workspaceFixture": "../testdata/edit", "verificationCommand": ["grep", "-R", "hello, world", "."]},
-
-    {"id": "fix-01", "name": "off-by-one", "class": "fix", "prompt": "In bugs.go the Sum function has an off-by-one error that skips the last element; fix the loop so it sums all elements.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestSumIncludesLastElement", "./..."]},
-    {"id": "fix-02", "name": "nil deref", "class": "fix", "prompt": "In bugs.go DefaultConfig returns nil and Port() dereferences it; make DefaultConfig return a valid Config with Port 8080.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestPortReturnsDefault", "./..."]},
-    {"id": "fix-03", "name": "wrong operator", "class": "fix", "prompt": "In bugs.go Max uses the wrong inequality and returns the smaller value; fix it to return the larger.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestMaxReturnsLarger", "./..."]},
-    {"id": "fix-04", "name": "truncated read", "class": "fix", "prompt": "In bugs.go ReadFirst returns only a single character instead of the full content; fix it to return the whole string.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestReadFirstReturnsFullContent", "./..."]},
-    {"id": "fix-05", "name": "swapped args", "class": "fix", "prompt": "In bugs.go Divide swaps its operands; fix the order so Divide(10,2) returns 5.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestDivideOrder", "./..."]},
-    {"id": "fix-06", "name": "missing return", "class": "fix", "prompt": "In bugs.go Classify is missing a return for the high case; add it so Classify(500) returns \"high\".", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestClassifyHigh", "./..."]},
-    {"id": "fix-07", "name": "wrong label prefix", "class": "fix", "prompt": "In bugs.go Label returns 'admin-<name>' but the test expects 'user-<name>'; change the prefix from 'admin-' to 'user-'.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-run", "TestLabelUsesString", "./..."]},
-    {"id": "fix-08", "name": "race on counter", "class": "fix", "prompt": "In bugs.go the global counter is incremented without synchronization; guard it with a mutex.", "workspaceFixture": "../testdata/fix", "verificationCommand": ["go", "test", "-race", "-run", "TestCounterRace", "./..."]},
-
-    {"id": "refactor-01", "name": "extract helper", "class": "refactor", "prompt": "Extract the duplicated greeting logic into a single helper function and call it from both sites.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-    {"id": "refactor-02", "name": "split a file", "class": "refactor", "prompt": "Split main.go into two files: one for Config, one for the rest, keeping the package building.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-    {"id": "refactor-03", "name": "rename package", "class": "refactor", "prompt": "Rename the package from app to zeroapp everywhere and keep it building.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-    {"id": "refactor-04", "name": "introduce a type", "class": "refactor", "prompt": "Introduce a named type for the map[string]int used in main.go and update its users.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-    {"id": "refactor-05", "name": "inline a wrapper", "class": "refactor", "prompt": "Inline the thin wrapper function into its single caller and remove it.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-    {"id": "refactor-06", "name": "consolidate errors", "class": "refactor", "prompt": "Consolidate the repeated error-construction blocks into a single helper.", "workspaceFixture": "../testdata/refactor", "verificationCommand": ["go", "build", "./..."]},
-
-    {"id": "longproc-01", "name": "long build", "class": "longproc", "prompt": "Run the build command and report whether it succeeds. Do not modify anything.", "workspaceFixture": "../testdata/longproc"},
-    {"id": "longproc-02", "name": "long test", "class": "longproc", "prompt": "Run go test ./... and report the result. Do not modify anything.", "workspaceFixture": "../testdata/longproc"},
-    {"id": "longproc-03", "name": "bench run", "class": "longproc", "prompt": "Run go test -bench=. -benchtime=1x and report the benchmark numbers. Do not modify anything.", "workspaceFixture": "../testdata/longproc"},
-    {"id": "longproc-04", "name": "vet run", "class": "longproc", "prompt": "Run go vet ./... and report the output. Do not modify anything.", "workspaceFixture": "../testdata/longproc"},
-
-    {"id": "longctx-01", "name": "summarize large file", "class": "longctx", "prompt": "Read the large generated file big.go and summarize its structure. Do not modify anything.", "workspaceFixture": "../testdata/longctx"},
-    {"id": "longctx-02", "name": "find in large file", "class": "longctx", "prompt": "In big.go, find every function that returns an error and list them. Do not modify anything.", "workspaceFixture": "../testdata/longctx"},
-    {"id": "longctx-03", "name": "cross-file question", "class": "longctx", "prompt": "Read all .go files and report which two functions share the same name across files. Do not modify anything.", "workspaceFixture": "../testdata/longctx"},
-    {"id": "longctx-04", "name": "trace through large file", "class": "longctx", "prompt": "Starting from Handle, trace the call path through big.go and report the chain. Do not modify anything.", "workspaceFixture": "../testdata/longctx"},
-
-    {"id": "parallel-01", "name": "read six files", "class": "parallel", "prompt": "Read a.txt, b.txt, c.txt, d.txt, e.txt, and f.txt, then report the first line of each. Do not modify anything.", "workspaceFixture": "../testdata/parallel"},
-    {"id": "parallel-02", "name": "grep six patterns", "class": "parallel", "prompt": "Search for 'alpha', 'beta', 'gamma', 'delta', 'epsilon', and 'zeta' across the repo and report which files contain each. Do not modify anything.", "workspaceFixture": "../testdata/parallel"},
-    {"id": "parallel-03", "name": "list six dirs", "class": "parallel", "prompt": "List the contents of dir1, dir2, dir3, dir4, dir5, and dir6. Do not modify anything.", "workspaceFixture": "../testdata/parallel"},
-    {"id": "parallel-04", "name": "stat six files", "class": "parallel", "prompt": "Report the line count of a.txt, b.txt, c.txt, d.txt, e.txt, and f.txt. Do not modify anything.", "workspaceFixture": "../testdata/parallel"},
-    {"id": "parallel-05", "name": "read configs", "class": "parallel", "prompt": "Read config1.json, config2.json, config3.json, config4.json, config5.json, and config6.json and report each top-level key. Do not modify anything.", "workspaceFixture": "../testdata/parallel"},
-    {"id": "parallel-06", "name": "summarize six", "class": "parallel", "prompt": "Read the six .txt files and give a one-line summary of each. Do not modify anything.", "workspaceFixture": "../testdata/parallel"}
-  ]
-}

internal/perfbench/reports/README.md

@@ -1,84 +0,0 @@
-# Per-turn benchmark reports
-
-This directory holds generated per-turn benchmark reports. It is **not** a
-checked-in source of truth for performance — the numbers are machine- and
-model-specific, so a single snapshot here would mislead rather than inform.
-
-## Generating a baseline
-
-Run the harness over the checked-in manifest:
-
-```sh
-make baseline ZERO_BENCH_MODEL=<model>              # uses ./zero
-make baseline ZERO_BENCH_MODEL=<model> ZERO_BENCH_BINARY=/path/to/zero
-```
-
-This builds `zero`, then runs `zero-perf-bench turn` over
-`internal/perfbench/manifests/baseline.json`, capturing each turn's trace
-(`zero exec --trace <tmpfile>`) and writing the aggregated result to
-`reports/baseline.json`.
-
-The JSON report is self-describing: model, mode, self-correct flag, version,
-commit, date, per-span median/P95, the **top three controllable latency sources**
-ranked by **exclusive** time, per-class roll-ups, and token/count totals.
-That top-three list is the Phase 0 baseline's "do not proceed until" criterion —
-it names where a turn actually spends time so later optimization work is
-targeted, not guessed.
-
-### Attribution model (honest by construction)
-
-Spans record wall intervals and are **not** summed into each other. Each span's
-**exclusive** time is its own duration minus the union of its nested children's
-intervals, derived at finish by interval containment. So a `provider_connect`
-that runs concurrently inside `generation`, or a `permission_wait` nested inside
-`tool_execution`, each contributes only its own exclusive time — they no longer
-double-count the same wall. The top-latency shares therefore sum to ~1 for a
-well-instrumented run, and the ranking reflects where wall time is actually
-spent.
-
-**Coverage** is the fraction of wall covered by the union of all span
-intervals (capped at 1.0) — the honest "≥95% of wall accounted for" metric. A
-run with `coverage < 0.95` has uninstrumented gaps, not an inflated attribution.
-
-### Pass/fail is reported per oracle tier
-
-Pass/fail is split into three tiers so it cannot be misread as a blanket
-correctness verdict (see `MANIFEST.md` for the class breakdown):
-
-- **Correctness** (`tasksVerified` / `tasksPassed` / `correctnessPassRate`):
-  tasks with a positive oracle — edit's substring grep, fix's scoped `go test`.
-  This is the only pass rate that can move with model quality.
-- **Build-only** (`buildCheckedTasks` / `buildPassedTasks` / `buildPassRate`):
-  refactor's `go build ./...`, which proves the edit compiles but not that the
-  refactor achieved its goal. Reported separately, never in `correctnessPassRate`.
-- **Latency-only** (`latencyOnlyTasks`): the read-only classes (nav, longproc,
-  longctx, parallel) carry no oracle — an exit 0 only proves the turn ran. They
-  contribute to latency and span attribution and are excluded from every pass
-  rate.
-
-`tasksAttempted` is still the total across all three tiers. The tier class lists
-are echoed in the report so a consumer can see exactly which classes each rate
-is computed over.
-
-> **Do not average the tier pass rates. Do not report a single "pass rate".**
-> `correctnessPassRate` and `buildPassRate` measure different things over
-> different task sets and must never be combined — a weighted or arithmetic
-> mean of the two is a number that means nothing. The schema offers no
-> headline pass rate on purpose: a consumer must name the tier it is reporting
-> (`correctness`, `build`, or `latency-only`) and quote that tier's fields. A
-> read-only run that exits 0 is a latency sample, not a pass.
-
-## What to commit
-
-Commit the manifest and fixtures (`manifests/`, `../testdata/`), not a generated
-`baseline.json`. A generated report belongs in a PR description or a shared
-dashboard as evidence for one configuration, not in the tree as a durable
-expectation. `.gitkeep` keeps the directory present between runs.
-
-## Caveats
-
-- Each task is a fresh `zero exec` process, so iterations are **cold-start**
-  samples. A warm path (reusing an in-process agent) is future work.
-- Mutating tasks (edit/fix/refactor) run against a per-invocation **copy** of
-  their fixture, so the checked-in fixtures stay clean and one task's edits
-  can't bleed into the next iteration.

internal/perfbench/taskbench.go

@@ -25,29 +25,17 @@ const TaskSchemaVersion = 1
 // ZERO must satisfy. The set is recorded by ID with every result so a published
 // number is traceable to the exact tasks that produced it.
 type TaskSet struct {
-	ID          string      `json:"id"`
-	Name        string      `json:"name,omitempty"`
-	Description string      `json:"description,omitempty"`
-	Tasks       []BenchTask `json:"tasks"`
-	// BuildOnlyClasses lists task classes whose verificationCommand is a
-	// non-positive build check (e.g. refactor's `go build ./...`): it proves the
-	// edit compiles, not that the refactor achieved its goal. The turn benchmark
-	// reports these separately from correctness oracles so a build-pass cannot be
-	// misread as a correctness pass. Classes with a verificationCommand that are
-	// NOT listed here are treated as correctness classes; classes whose tasks
-	// carry no verificationCommand are latency-only regardless of this list.
-	BuildOnlyClasses []string `json:"buildOnlyClasses,omitempty"`
+	ID    string      `json:"id"`
+	Name  string      `json:"name,omitempty"`
+	Tasks []BenchTask `json:"tasks"`
 }
 
 // BenchTask is one benchmark task. WorkspaceFixture is the relative path of the
 // task's starting workspace; VerificationCommand (optional) is the command the
-// default runner executes to decide pass/fail after ZERO finishes. Class groups
-// the task for the turn-benchmark's per-group latency breakdown (e.g. "nav",
-// "edit", "fix"); it is optional and ignored by the pass/fail runner.
+// default runner executes to decide pass/fail after ZERO finishes.
 type BenchTask struct {
 	ID                  string   `json:"id"`
 	Name                string   `json:"name,omitempty"`
-	Class               string   `json:"class,omitempty"`
 	Prompt              string   `json:"prompt"`
 	WorkspaceFixture    string   `json:"workspaceFixture,omitempty"`
 	VerificationCommand []string `json:"verificationCommand,omitempty"`

internal/perfbench/testdata/edit/main.go

@@ -1,42 +0,0 @@
-// Package edit is a small workspace for the edit-class benchmark tasks: each
-// task asks the agent to make one targeted edit, verified by a grep/build
-// command in the manifest.
-package edit
-
-import (
-	"fmt"
-	"os"
-)
-
-// Port is the default listen port.
-const Port = 8080
-
-// MaxRetries is the maximum number of retries before giving up.
-const MaxRetries = 3
-
-// Config holds runtime configuration.
-type Config struct {
-	Name string
-}
-
-// greet returns a greeting for the named user.
-//
-// This receieves a name and formats a hello message.
-func greet(name string) string {
-	return "hello"
-}
-
-// load reads a config file and returns its bytes, or an error on failure.
-func load(path string) ([]byte, error) {
-	data, err := os.ReadFile(path)
-	if err != nil {
-		return nil, err
-	}
-	return data, nil
-}
-
-func main() {
-	fmt.Println("debug: starting")
-	cfg := Config{Name: "demo"}
-	fmt.Println(greet(cfg.Name))
-}

internal/perfbench/testdata/edit/version.go

@@ -1,4 +0,0 @@
-package edit
-
-// Version is the build version string.
-const Version = "1.0.0"

internal/perfbench/testdata/fix/bugs.go

@@ -1,89 +0,0 @@
-// Package fix holds deliberately buggy functions for the bug-fix benchmark
-// tasks. Each function is covered by its own test (see bugs_test.go) and each
-// task verifies with `go test -run <TestName>`, so the tasks are independent:
-// the initial package compiles and the targeted test fails until the named bug
-// is fixed. Every bug is fixable without changing a function signature, so the
-// test files compile in both the buggy and the fixed state.
-package fix
-
-import "fmt"
-
-// Sum returns the sum of all elements. BUG: off-by-one — skips the last element.
-func Sum(xs []int) int {
-	total := 0
-	for i := 0; i < len(xs)-1; i++ {
-		total += xs[i]
-	}
-	return total
-}
-
-// DefaultConfig returns the default configuration. BUG: returns nil, which
-// Port() then dereferences.
-func DefaultConfig() *Config {
-	return nil
-}
-
-// Config holds demo settings.
-type Config struct {
-	Port int
-}
-
-// Port returns the configured port. BUG: panics when DefaultConfig() is nil.
-func Port() int {
-	return DefaultConfig().Port
-}
-
-// Max returns the larger of a and b. BUG: uses the wrong inequality and returns
-// the smaller value.
-func Max(a, b int) int {
-	if a < b {
-		return a
-	}
-	return b
-}
-
-// ReadFirst returns the first byte of the given file's content as a string. BUG:
-// returns only a single character instead of the full content.
-func ReadFirst(content string) string {
-	if len(content) == 0 {
-		return ""
-	}
-	return string(content[0])
-}
-
-// Divide returns a divided by b. BUG: the operands are swapped.
-func Divide(a, b int) int {
-	return b / a
-}
-
-// Classify returns "low", "mid", or "high". BUG: the "high" branch is missing a
-// return, so high values fall through to the default "".
-func Classify(n int) string {
-	if n < 10 {
-		return "low"
-	}
-	if n < 100 {
-		return "mid"
-	}
-	// BUG: missing return for the high case
-	return ""
-}
-
-// Label returns a display label for the given name. BUG: uses the wrong prefix
-// ("admin-" instead of "user-"), producing the wrong text.
-func Label(name string) string {
-	return fmt.Sprintf("admin-%s", name)
-}
-
-// counter is a shared, unsynchronized counter. BUG: Inc is not goroutine-safe.
-var counter int
-
-// Inc increments the shared counter.
-func Inc() {
-	counter++
-}
-
-// Count returns the current counter value.
-func Count() int {
-	return counter
-}

internal/perfbench/testdata/fix/bugs_test.go

@@ -1,68 +0,0 @@
-package fix
-
-import (
-	"sync"
-	"testing"
-)
-
-func TestSumIncludesLastElement(t *testing.T) {
-	if got := Sum([]int{1, 2, 3, 4}); got != 10 {
-		t.Fatalf("Sum = %d, want 10", got)
-	}
-}
-
-func TestPortReturnsDefault(t *testing.T) {
-	if got := Port(); got != 8080 {
-		t.Fatalf("Port() = %d, want 8080 (DefaultConfig returned nil)", got)
-	}
-}
-
-func TestMaxReturnsLarger(t *testing.T) {
-	if got := Max(3, 7); got != 7 {
-		t.Fatalf("Max(3,7) = %d, want 7", got)
-	}
-}
-
-func TestReadFirstReturnsFullContent(t *testing.T) {
-	if got := ReadFirst("hello"); got != "hello" {
-		t.Fatalf("ReadFirst = %q, want %q", got, "hello")
-	}
-}
-
-func TestDivideOrder(t *testing.T) {
-	if got := Divide(10, 2); got != 5 {
-		t.Fatalf("Divide(10,2) = %d, want 5", got)
-	}
-}
-
-func TestClassifyHigh(t *testing.T) {
-	if got := Classify(500); got != "high" {
-		t.Fatalf("Classify(500) = %q, want %q", got, "high")
-	}
-}
-
-func TestLabelUsesString(t *testing.T) {
-	if got := Label("alice"); got != "user-alice" {
-		t.Fatalf("Label = %q, want %q", got, "user-alice")
-	}
-}
-
-// TestCounterRace exercises Inc concurrently. Run with `go test -race`:
-// the unsynchronized counter trips the race detector until a mutex is added.
-func TestCounterRace(t *testing.T) {
-	counter = 0
-	var wg sync.WaitGroup
-	for i := 0; i < 200; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			for j := 0; j < 100; j++ {
-				Inc()
-			}
-		}()
-	}
-	wg.Wait()
-	if got := Count(); got != 20000 {
-		t.Fatalf("Count = %d, want 20000", got)
-	}
-}

internal/perfbench/testdata/longproc/main.go

@@ -1,21 +0,0 @@
-// Package longproc is a small buildable workspace for the long-running-process
-// benchmark tasks, which run build/test/vet/bench commands against it.
-package longproc
-
-import "fmt"
-
-// Version is the demo build's version string.
-const Version = "1.0.0"
-
-// Process simulates a bounded unit of work for the benchmark task.
-func Process(n int) int {
-	total := 0
-	for i := 0; i < n; i++ {
-		total += i
-	}
-	return total
-}
-
-func main() {
-	fmt.Println("longproc fixture", Version, Process(100))
-}

internal/perfbench/testdata/longproc/main_test.go

@@ -1,21 +0,0 @@
-package longproc
-
-import "testing"
-
-// benchSink keeps Process(100) live across the benchmark loop so the compiler
-// can't optimize the call away and report an unrealistically fast benchmark.
-var benchSink int
-
-func TestProcess(t *testing.T) {
-	if got := Process(100); got != 4950 {
-		t.Fatalf("Process(100) = %d, want 4950", got)
-	}
-}
-
-func BenchmarkProcess(b *testing.B) {
-	var sink int
-	for i := 0; i < b.N; i++ {
-		sink = Process(100)
-	}
-	benchSink = sink
-}

internal/perfbench/testdata/nav/README.md

@@ -1,3 +0,0 @@
-# nav fixture
-
-Small read-only workspace for navigation/classification benchmark tasks.

internal/perfbench/testdata/nav/config.json

@@ -1,5 +0,0 @@
-{
-  "port": 8080,
-  "name": "nav-fixture",
-  "retries": 3
-}

internal/perfbench/testdata/nav/main.go

@@ -1,24 +0,0 @@
-// Package nav is a small read-only fixture for navigation tasks.
-package nav
-
-import "fmt"
-
-// MaxRetries is the configured retry limit for the demo client.
-const MaxRetries = 3
-
-// Config holds the demo client's settings.
-type Config struct {
-	Port int
-	Name string
-}
-
-// greet returns a greeting for the given name.
-// receieves is a deliberate typo present in the fixture (see edit tasks).
-func greet(name string) string {
-	return fmt.Sprintf("hello, %s", name)
-}
-
-// main is the fixture entry point.
-func main() {
-	fmt.Println(greet("world"))
-}

internal/perfbench/testdata/parallel/a.txt

@@ -1,4 +0,0 @@
-first line of a.txt
-second line of a.txt
-third line of a.txt
-alpha alpha alpha

internal/perfbench/testdata/parallel/b.txt

@@ -1,4 +0,0 @@
-first line of b.txt
-second line of b.txt
-third line of b.txt
-beta beta beta

internal/perfbench/testdata/parallel/c.txt

@@ -1,4 +0,0 @@
-first line of c.txt
-second line of c.txt
-third line of c.txt
-gamma gamma gamma

internal/perfbench/testdata/parallel/config1.json

@@ -1 +0,0 @@
-{ "id": 1, "key": "value-1", "enabled": true }

internal/perfbench/testdata/parallel/config2.json

@@ -1 +0,0 @@
-{ "id": 2, "key": "value-2", "enabled": true }

internal/perfbench/testdata/parallel/config3.json

@@ -1 +0,0 @@
-{ "id": 3, "key": "value-3", "enabled": true }

internal/perfbench/testdata/parallel/config4.json

@@ -1 +0,0 @@
-{ "id": 4, "key": "value-4", "enabled": true }

internal/perfbench/testdata/parallel/config5.json

@@ -1 +0,0 @@
-{ "id": 5, "key": "value-5", "enabled": true }

internal/perfbench/testdata/parallel/config6.json

@@ -1 +0,0 @@
-{ "id": 6, "key": "value-6", "enabled": true }

internal/perfbench/testdata/parallel/d.txt

@@ -1,4 +0,0 @@
-first line of d.txt
-second line of d.txt
-third line of d.txt
-delta delta delta

internal/perfbench/testdata/parallel/dir1/notes.md

@@ -1 +0,0 @@
-contents of dir1/notes.md

internal/perfbench/testdata/parallel/dir2/notes.md

@@ -1 +0,0 @@
-contents of dir2/notes.md

internal/perfbench/testdata/parallel/dir3/notes.md

@@ -1 +0,0 @@
-contents of dir3/notes.md

internal/perfbench/testdata/parallel/dir4/notes.md

@@ -1 +0,0 @@
-contents of dir4/notes.md

internal/perfbench/testdata/parallel/dir5/notes.md

@@ -1 +0,0 @@
-contents of dir5/notes.md

internal/perfbench/testdata/parallel/dir6/notes.md

@@ -1 +0,0 @@
-contents of dir6/notes.md

internal/perfbench/testdata/parallel/e.txt

@@ -1,4 +0,0 @@
-first line of e.txt
-second line of e.txt
-third line of e.txt
-epsilon epsilon epsilon

internal/perfbench/testdata/parallel/f.txt

@@ -1,4 +0,0 @@
-first line of f.txt
-second line of f.txt
-third line of f.txt
-zeta zeta zeta

internal/perfbench/testdata/refactor/main.go

@@ -1,55 +0,0 @@
-// Package refactor is the starting workspace for the multi-file refactor
-// benchmark tasks. It is intentionally a little messy (duplicated logic, a
-// bare map type, a thin wrapper) so the refactor prompts have real work to do.
-// It builds cleanly at all times; the tasks keep it building.
-package refactor
-
-import "fmt"
-
-// Config holds demo settings.
-type Config struct {
-	Name string
-	Port int
-}
-
-// GreetFromConfig returns a greeting using the config's name.
-func GreetFromConfig(c Config) string {
-	return fmt.Sprintf("hello, %s", c.Name)
-}
-
-// GreetByName returns a greeting using a bare name. BUG-note: this duplicates
-// GreetFromConfig's formatting — refactor-01 extracts a shared helper.
-func GreetByName(name string) string {
-	return fmt.Sprintf("hello, %s", name)
-}
-
-// Wrapper is a thin wrapper around GreetByName — refactor-05 inlines it.
-func Wrapper(name string) string {
-	return GreetByName(name)
-}
-
-// stats is a bare map used across the file — refactor-04 introduces a named type.
-var stats = map[string]int{}
-
-// Record bumps a stat by name.
-func Record(name string) {
-	stats[name]++
-}
-
-// Lookup returns a stat by name.
-func Lookup(name string) int {
-	return stats[name]
-}
-
-// load failed wrapped error — refactor-06 consolidates these.
-func firstError() error {
-	return fmt.Errorf("load failed: missing input")
-}
-
-func secondError() error {
-	return fmt.Errorf("load failed: bad format")
-}
-
-func buildError(ctx string) error {
-	return fmt.Errorf("load failed: %s", ctx)
-}

internal/perfbench/turn_bench.go

@@ -1,679 +0,0 @@
-package perfbench
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"errors"
-	"fmt"
-	"io"
-	"io/fs"
-	"os"
-	"os/exec"
-	"path/filepath"
-	"sort"
-	"strings"
-	"time"
-
-	"github.com/Gitlawb/zero/internal/trace"
-)
-
-// TurnSchemaVersion is the schema version of a published turn-benchmark result.
-// Bump when the TurnBenchResult shape changes so consumers can detect drift.
-//
-// v2 splits pass/fail into three oracle tiers so the report cannot be misread as
-// a blanket correctness verdict: tasksVerified/tasksPassed/correctnessPassRate
-// cover only the positive-oracle classes (edit/fix); buildCheckedTasks/
-// buildPassedTasks/buildPassRate cover the non-positive build-check classes
-// (refactor); latencyOnlyTasks covers the no-oracle classes (nav/longproc/
-// longctx/parallel). tasksAttempted is still every task run.
-const TurnSchemaVersion = 2
-
-// TurnRunner runs one benchmark task and reports its outcome plus the captured
-// per-turn trace. A non-nil Err means the run failed to execute (process crash);
-// Passed reflects the verification result. Trace is the parsed NDJSON trace
-// (nil when the run errored before emitting one).
-type TurnRunner func(ctx context.Context, task BenchTask, rc RunContext) TurnTaskOutcome
-
-// TurnTaskOutcome is what a TurnRunner reports for one task iteration.
-type TurnTaskOutcome struct {
-	Passed    bool
-	VerifyErr string
-	WallMs    float64
-	Trace     *trace.TurnTrace
-	// TraceIssue, when non-empty, explains why the per-turn trace could not be
-	// parsed (file missing or malformed). The run still has a pass/fail verdict,
-	// but an incomplete measurement is surfaced as a result warning so it can't
-	// masquerade as a clean attribution sample.
-	TraceIssue string
-	Err        error
-}
-
-// TurnBenchConfig configures a turn-benchmark run.
-type TurnBenchConfig struct {
-	Model       string
-	Mode        string
-	SelfCorrect bool
-	Version     string
-	Commit      string
-	// Iterations is how many times each task is run. The per-process `zero exec`
-	// runner is inherently cold-start, so this is the sample count for per-span
-	// median/P95 — a genuine warm path needs an in-process runner (future work).
-	Iterations int
-	// Runner executes one task iteration. Required.
-	Runner TurnRunner
-	// Now overrides the clock for the recorded date (tests inject a fixed time).
-	Now func() time.Time
-}
-
-// SpanStats summarizes one span's duration across all measured task iterations.
-type SpanStats struct {
-	Count    int     `json:"count"`
-	TotalMs  float64 `json:"totalMs"`
-	MedianMs float64 `json:"medianMs"`
-	P95Ms    float64 `json:"p95Ms"`
-	MaxMs    float64 `json:"maxMs"`
-}
-
-// LatencySource is one of the top controllable latency sources, ranked by total
-// exclusive time across the whole run. Share is its fraction of total exclusive
-// span time. Because exclusive time subtracts nested children, concurrent and
-// nested spans no longer double-count, so shares of the top sources sum to ~1
-// for a well-instrumented run.
-type LatencySource struct {
-	Span    string  `json:"span"`
-	TotalMs float64 `json:"totalMs"`
-	Share   float64 `json:"share"`
-}
-
-// ClassSummary is the per-class (task group) roll-up.
-//
-// Passed is the count that passed THIS class's oracle: a correctness pass for
-// edit/fix, a build pass for refactor, and always 0 for the no-oracle classes
-// (nav/longproc/longctx/parallel). Verified is how many tasks in the class
-// carry an oracle (correctness or build); LatencyOnly is how many carry none.
-// A latency-only class therefore reports Passed=0, Verified=0, LatencyOnly=Tasks.
-type ClassSummary struct {
-	Tasks       int                `json:"tasks"`
-	Verified    int                `json:"verified"`
-	Passed      int                `json:"passed"`
-	LatencyOnly int                `json:"latencyOnly"`
-	WallMs      NumericStats       `json:"wallMs"`
-	SpanTotals  map[string]float64 `json:"spanTotals"`
-}
-
-// TurnBenchResult is the publishable turn-benchmark record.
-//
-// Pass/fail is split into three oracle tiers so it cannot be misread as a
-// blanket correctness verdict:
-//   - Correctness (tasksVerified / tasksPassed / correctnessPassRate): tasks
-//     with a positive oracle (edit/fix). This is the only pass rate that can
-//     move with model quality.
-//   - Build-only (buildCheckedTasks / buildPassedTasks / buildPassRate): tasks
-//     whose oracle is a non-positive build check (refactor `go build`). A pass
-//     means it compiles, not that the refactor is correct.
-//   - Latency-only (latencyOnlyTasks): tasks with no oracle (nav/longproc/
-//     longctx/parallel). They ran for latency and span attribution only and are
-//     excluded from every pass rate.
-//
-// tasksAttempted is still the total number of tasks run across all three tiers.
-// The tier class lists are echoed so a consumer can see exactly which classes
-// each rate is computed over.
-type TurnBenchResult struct {
-	SchemaVersion       int                     `json:"schemaVersion"`
-	Suite               string                  `json:"suite"`
-	Model               string                  `json:"model"`
-	Mode                string                  `json:"mode,omitempty"`
-	SelfCorrect         bool                    `json:"selfCorrect"`
-	Version             string                  `json:"version,omitempty"`
-	Commit              string                  `json:"commit,omitempty"`
-	Date                string                  `json:"date"`
-	TasksAttempted      int                     `json:"tasksAttempted"`
-	TasksVerified       int                     `json:"tasksVerified"`
-	TasksPassed         int                     `json:"tasksPassed"`
-	LatencyOnlyTasks    int                     `json:"latencyOnlyTasks"`
-	BuildCheckedTasks   int                     `json:"buildCheckedTasks"`
-	BuildPassedTasks    int                     `json:"buildPassedTasks"`
-	CorrectnessPassRate float64                 `json:"correctnessPassRate"`
-	BuildPassRate       float64                 `json:"buildPassRate"`
-	CorrectnessClasses  []string                `json:"correctnessClasses,omitempty"`
-	BuildOnlyClasses    []string                `json:"buildOnlyClasses,omitempty"`
-	LatencyOnlyClasses  []string                `json:"latencyOnlyClasses,omitempty"`
-	Iterations          int                     `json:"iterations"`
-	PerSpan             map[string]SpanStats    `json:"perSpan"`
-	TopLatency          []LatencySource         `json:"topLatency"`
-	PerClass            map[string]ClassSummary `json:"perClass"`
-	Totals              TurnBenchTotals         `json:"totals"`
-	Warnings            []Warning               `json:"warnings,omitempty"`
-}
-
-// TurnBenchTotals aggregates token and count totals across the whole run.
-type TurnBenchTotals struct {
-	InputTokens       int64 `json:"inputTokens"`
-	CachedInputTokens int64 `json:"cachedInputTokens"`
-	OutputTokens      int64 `json:"outputTokens"`
-	ModelRequests     int64 `json:"modelRequests"`
-	ToolCalls         int64 `json:"toolCalls"`
-	Retries           int64 `json:"retries"`
-	Reconnects        int64 `json:"reconnects"`
-	Compactions       int64 `json:"compactions"`
-}
-
-// RunTurnBench executes every task in the set with the configured runner and
-// returns a self-describing per-turn result. It never aborts on a single task
-// failure — every task is attempted and recorded. Per-span stats aggregate
-// across iterations; the top three controllable latency sources are ranked by
-// total attributed time.
-func RunTurnBench(ctx context.Context, set TaskSet, cfg TurnBenchConfig) (TurnBenchResult, error) {
-	if len(set.Tasks) == 0 {
-		return TurnBenchResult{}, errors.New("task set has no tasks")
-	}
-	if strings.TrimSpace(cfg.Model) == "" {
-		return TurnBenchResult{}, errors.New("turn benchmark requires a model")
-	}
-	if cfg.Runner == nil {
-		return TurnBenchResult{}, errors.New("turn benchmark requires a runner")
-	}
-	iterations := cfg.Iterations
-	if iterations < 1 {
-		iterations = 1
-	}
-	now := cfg.Now
-	if now == nil {
-		now = time.Now
-	}
-	rc := RunContext{Model: cfg.Model, Mode: cfg.Mode, SelfCorrect: cfg.SelfCorrect}
-
-	perSpanSamples := map[string][]float64{}
-	classWalls := map[string][]float64{}
-	classSpanTotals := map[string]map[string]float64{}
-	classTasks := map[string]int{}
-	classVerified := map[string]int{}
-	classPassed := map[string]int{}
-	classLatencyOnly := map[string]int{}
-	correctnessClasses := map[string]bool{}
-	buildOnlyClasses := map[string]bool{}
-	latencyOnlyClasses := map[string]bool{}
-	var totals TurnBenchTotals
-
-	// A class is build-only when the manifest declares it in BuildOnlyClasses.
-	// A task's tier is then decided per-task: no verificationCommand => latency-
-	// only; otherwise build-only if its class is declared, else correctness. The
-	// latency-only tier is always driven by oracle presence, never by the
-	// declared list, so a declared build-only class with a missing oracle still
-	// counts as latency-only rather than silently passing on exit 0.
-	buildOnly := map[string]bool{}
-	for _, c := range set.BuildOnlyClasses {
-		buildOnly[strings.TrimSpace(c)] = true
-	}
-
-	result := TurnBenchResult{
-		SchemaVersion: TurnSchemaVersion,
-		Suite:         strings.TrimSpace(set.ID),
-		Model:         strings.TrimSpace(cfg.Model),
-		Mode:          strings.TrimSpace(cfg.Mode),
-		SelfCorrect:   cfg.SelfCorrect,
-		Version:       strings.TrimSpace(cfg.Version),
-		Commit:        strings.TrimSpace(cfg.Commit),
-		Date:          now().UTC().Format(time.RFC3339),
-		Iterations:    iterations,
-		PerSpan:       map[string]SpanStats{},
-		PerClass:      map[string]ClassSummary{},
-	}
-
-	for _, task := range set.Tasks {
-		if err := ctx.Err(); err != nil {
-			return result, err
-		}
-		class := strings.TrimSpace(task.Class)
-		if class == "" {
-			class = "default"
-		}
-		classTasks[class]++
-		// A task counts as passed only when every iteration passed. The per-process
-		// runner is cold-start, so a flaky pass on one iteration and a fail on
-		// another is a real regression signal, not noise to average away.
-		passedForTask := true
-		for iter := 0; iter < iterations; iter++ {
-			outcome := cfg.Runner(ctx, task, rc)
-			if outcome.TraceIssue != "" {
-				result.Warnings = append(result.Warnings, Warning{
-					Metric:  "trace",
-					Message: fmt.Sprintf("task %s: %s", task.ID, outcome.TraceIssue),
-				})
-			}
-			if outcome.Err != nil {
-				// A crashed run must not look like a normal sample that's merely
-				// absent — surface it as a warning so a run that died every
-				// iteration can't pass as "fewer measurements."
-				result.Warnings = append(result.Warnings, Warning{
-					Metric:  "run",
-					Message: fmt.Sprintf("task %s: %v", task.ID, outcome.Err),
-				})
-				passedForTask = false
-				continue
-			}
-			if !outcome.Passed {
-				passedForTask = false
-			}
-			wall := outcome.WallMs
-			if wall <= 0 && outcome.Trace != nil {
-				wall = float64(outcome.Trace.WallDuration().Microseconds()) / 1000
-			}
-			if wall > 0 {
-				classWalls[class] = append(classWalls[class], wall)
-			}
-			if outcome.Trace != nil {
-				aggregateTotals(&totals, outcome.Trace)
-				for _, span := range outcome.Trace.Spans {
-					// Rank by exclusive time (duration minus nested children) so
-					// concurrent/nested spans do not double-count: provider_connect
-					// inside generation and permission_wait inside tool_execution
-					// each contribute their own exclusive time, not their parent's.
-					// A span whose exclusive is legitimately zero (a parent fully
-					// covered by its children) contributes zero on purpose — do
-					// NOT fall back to Duration, which would re-introduce the
-					// double-count. ReadNDJSON preserves a written exclusive_ms:0
-					// as 0 and only falls back to Duration when the key is absent,
-					// so span.Exclusive is always populated here.
-					ms := float64(span.Exclusive.Microseconds()) / 1000
-					perSpanSamples[span.Name] = append(perSpanSamples[span.Name], ms)
-					if classSpanTotals[class] == nil {
-						classSpanTotals[class] = map[string]float64{}
-					}
-					classSpanTotals[class][span.Name] += ms
-				}
-			}
-		}
-		result.TasksAttempted++
-
-		// Classify the task into an oracle tier and update only that tier's
-		// counters. A latency-only task (no verificationCommand) is never counted
-		// in any pass rate even when the runner reports Passed — an exit-0
-		// read-only run proves the turn ran, not that the answer was right.
-		hasOracle := len(task.VerificationCommand) > 0
-		switch {
-		case !hasOracle:
-			result.LatencyOnlyTasks++
-			classLatencyOnly[class]++
-			latencyOnlyClasses[class] = true
-		case buildOnly[class]:
-			result.BuildCheckedTasks++
-			classVerified[class]++
-			buildOnlyClasses[class] = true
-			if passedForTask {
-				result.BuildPassedTasks++
-				classPassed[class]++
-			}
-		default:
-			result.TasksVerified++
-			classVerified[class]++
-			correctnessClasses[class] = true
-			if passedForTask {
-				result.TasksPassed++
-				classPassed[class]++
-			}
-		}
-	}
-
-	for name, samples := range perSpanSamples {
-		result.PerSpan[name] = summarizeSpan(samples)
-	}
-	result.TopLatency = topLatencySources(result.PerSpan, 3)
-	for class := range classTasks {
-		walls := classWalls[class]
-		var wallStats NumericStats
-		if len(walls) > 0 {
-			wallStats = SummarizeSamples(walls)
-		}
-		result.PerClass[class] = ClassSummary{
-			Tasks:       classTasks[class],
-			Verified:    classVerified[class],
-			Passed:      classPassed[class],
-			LatencyOnly: classLatencyOnly[class],
-			WallMs:      wallStats,
-			SpanTotals:  classSpanTotals[class],
-		}
-	}
-	result.CorrectnessClasses = sortedSetKeys(correctnessClasses)
-	result.BuildOnlyClasses = sortedSetKeys(buildOnlyClasses)
-	result.LatencyOnlyClasses = sortedSetKeys(latencyOnlyClasses)
-	result.CorrectnessPassRate = passRate(result.TasksPassed, result.TasksVerified)
-	result.BuildPassRate = passRate(result.BuildPassedTasks, result.BuildCheckedTasks)
-	result.Totals = totals
-	return result, nil
-}
-
-// passRate is passed/total rounded to the benchmark's metric precision, or 0
-// when the denominator is zero (no tasks in that tier were run).
-func passRate(passed, total int) float64 {
-	if total <= 0 {
-		return 0
-	}
-	return RoundMetric(float64(passed) / float64(total))
-}
-
-func sortedSetKeys(set map[string]bool) []string {
-	if len(set) == 0 {
-		return nil
-	}
-	keys := make([]string, 0, len(set))
-	for k := range set {
-		keys = append(keys, k)
-	}
-	sort.Strings(keys)
-	return keys
-}
-
-func summarizeSpan(samples []float64) SpanStats {
-	if len(samples) == 0 {
-		return SpanStats{}
-	}
-	stats := SummarizeSamples(samples)
-	total := 0.0
-	for _, s := range samples {
-		total += s
-	}
-	return SpanStats{
-		Count:    len(samples),
-		TotalMs:  RoundMetric(total),
-		MedianMs: stats.Median,
-		P95Ms:    stats.P95,
-		MaxMs:    stats.Max,
-	}
-}
-
-func topLatencySources(perSpan map[string]SpanStats, top int) []LatencySource {
-	sources := make([]LatencySource, 0, len(perSpan))
-	totalAttributed := 0.0
-	for _, s := range perSpan {
-		totalAttributed += s.TotalMs
-	}
-	for name, s := range perSpan {
-		share := 0.0
-		if totalAttributed > 0 {
-			share = s.TotalMs / totalAttributed
-		}
-		sources = append(sources, LatencySource{
-			Span:    name,
-			TotalMs: s.TotalMs,
-			Share:   RoundMetric(share),
-		})
-	}
-	sort.SliceStable(sources, func(i, j int) bool {
-		if sources[i].TotalMs != sources[j].TotalMs {
-			return sources[i].TotalMs > sources[j].TotalMs
-		}
-		return sources[i].Span < sources[j].Span
-	})
-	if top > 0 && len(sources) > top {
-		sources = sources[:top]
-	}
-	return sources
-}
-
-func aggregateTotals(totals *TurnBenchTotals, tr *trace.TurnTrace) {
-	totals.InputTokens += tr.Counter(trace.CounterInputTokens)
-	totals.CachedInputTokens += tr.Counter(trace.CounterCachedInputTokens)
-	totals.OutputTokens += tr.Counter(trace.CounterOutputTokens)
-	totals.ModelRequests += tr.Counter(trace.CounterModelRequests)
-	totals.ToolCalls += tr.Counter(trace.CounterToolCalls)
-	totals.Retries += tr.Counter(trace.CounterRetryCount)
-	totals.Reconnects += tr.Counter(trace.CounterReconnectCount)
-	totals.Compactions += tr.Counter(trace.CounterCompactionCount)
-}
-
-// FormatTurnBenchSummary renders a human-readable turn-benchmark summary that
-// names the top controllable latency sources — the baseline's "do not proceed
-// until" criterion.
-func FormatTurnBenchSummary(result TurnBenchResult) string {
-	lines := []string{
-		"Zero turn benchmark: " + displayOrUnknown(result.Suite),
-		"model: " + displayOrUnknown(result.Model),
-		// The headline separates the three oracle tiers so an exit-0 read-only
-		// task can never inflate a "pass rate" that reads as correctness:
-		// correctness (positive oracle, edit/fix), build (non-positive `go build`,
-		// refactor), and latency-only (no oracle, nav/longproc/longctx/parallel).
-		fmt.Sprintf("tasks: %d total | correctness %d/%d (%.0f%%) | build %d/%d (%.0f%%) | latency-only %d | %d iter",
-			result.TasksAttempted,
-			result.TasksPassed, result.TasksVerified, result.CorrectnessPassRate*100,
-			result.BuildPassedTasks, result.BuildCheckedTasks, result.BuildPassRate*100,
-			result.LatencyOnlyTasks, result.Iterations),
-	}
-	if result.Mode != "" {
-		lines = append(lines, "mode: "+result.Mode)
-	}
-	if len(result.TopLatency) > 0 {
-		lines = append(lines, "top latency sources:")
-		for _, src := range result.TopLatency {
-			lines = append(lines, fmt.Sprintf("  %-18s %10s  %5.1f%%", src.Span, FormatMetric(src.TotalMs, "ms"), src.Share*100))
-		}
-	}
-	lines = append(lines, fmt.Sprintf("totals: in=%d (cached %d) out=%d | requests=%d tools=%d retries=%d reconnects=%d compactions=%d",
-		result.Totals.InputTokens, result.Totals.CachedInputTokens, result.Totals.OutputTokens,
-		result.Totals.ModelRequests, result.Totals.ToolCalls, result.Totals.Retries,
-		result.Totals.Reconnects, result.Totals.Compactions))
-	for _, class := range sortedClasses(result.PerClass) {
-		summary := result.PerClass[class]
-		median := FormatMetric(summary.WallMs.Median, "ms")
-		tier := classTier(result, class)
-		lines = append(lines, fmt.Sprintf("  [%s/%s] %d/%d passed, %d latency-only, wall median %s",
-			class, tier, summary.Passed, summary.Verified, summary.LatencyOnly, median))
-	}
-	return strings.Join(lines, "\n")
-}
-
-// classTier returns the oracle tier label a class was classified into, so the
-// per-class roll-up states explicitly which oracle (if any) its "passed" count
-// is measured against.
-func classTier(result TurnBenchResult, class string) string {
-	for _, c := range result.BuildOnlyClasses {
-		if c == class {
-			return "build"
-		}
-	}
-	for _, c := range result.LatencyOnlyClasses {
-		if c == class {
-			return "latency"
-		}
-	}
-	return "correctness"
-}
-
-func sortedClasses(perClass map[string]ClassSummary) []string {
-	classes := make([]string, 0, len(perClass))
-	for c := range perClass {
-		classes = append(classes, c)
-	}
-	sort.Strings(classes)
-	return classes
-}
-
-// WriteTurnBenchJSON writes the indented JSON form of a turn-benchmark result.
-func WriteTurnBenchJSON(w io.Writer, result TurnBenchResult) error {
-	encoder := json.NewEncoder(w)
-	encoder.SetIndent("", "  ")
-	return encoder.Encode(result)
-}
-
-// NewTurnExecRunner builds the production turn-benchmark runner: it invokes
-// headless `zero exec` with stream-json output AND `--trace <tmpfile>`, then
-// parses the emitted NDJSON trace into a *trace.TurnTrace. binary is the path to
-// the `zero` binary; extraArgs are appended to every invocation. Pass/fail is
-// decided from the stream-json run_end exit code (and the task's
-// VerificationCommand when present), exactly like NewExecRunner.
-func NewTurnExecRunner(binary string, extraArgs ...string) TurnRunner {
-	return func(ctx context.Context, task BenchTask, rc RunContext) TurnTaskOutcome {
-		// Isolate the workspace: copy the fixture into a fresh temp dir so a
-		// mutating task (edit/fix/refactor) can't dirty the shared, checked-in
-		// fixture or bleed into a later iteration of the same task. When no
-		// fixture is configured the agent runs in the caller's cwd as before.
-		if fixture := strings.TrimSpace(task.WorkspaceFixture); fixture != "" {
-			copyDir, cerr := copyFixture(fixture)
-			if cerr != nil {
-				return TurnTaskOutcome{Err: fmt.Errorf("isolate fixture: %w", cerr)}
-			}
-			defer os.RemoveAll(copyDir)
-			task.WorkspaceFixture = copyDir
-		}
-
-		traceFile, err := os.CreateTemp("", "zero-turn-trace-*.ndjson")
-		if err != nil {
-			return TurnTaskOutcome{Err: fmt.Errorf("create trace file: %w", err)}
-		}
-		_ = traceFile.Close()
-		tracePath := traceFile.Name()
-		defer os.Remove(tracePath)
-
-		args := buildTurnExecArgs(task, rc, tracePath, extraArgs)
-		cmd := exec.CommandContext(ctx, binary, args...)
-		cmd.Env = appendNoColor(os.Environ())
-		if dir := strings.TrimSpace(task.WorkspaceFixture); dir != "" {
-			cmd.Dir = dir
-		}
-		var outBuf, errBuf bytes.Buffer
-		cmd.Stdout = &outBuf
-		cmd.Stderr = &errBuf
-		start := time.Now()
-		runErr := cmd.Run()
-		wallMs := float64(time.Since(start).Microseconds()) / 1000
-
-		exitCode, haveExit := streamJSONExitCode(outBuf.Bytes())
-		outcome := TurnTaskOutcome{WallMs: wallMs}
-		if haveExit && exitCode != 0 {
-			outcome.VerifyErr = fmt.Sprintf("agent run_end exit code %d", exitCode)
-		} else if !haveExit {
-			detail := strings.TrimSpace(errBuf.String())
-			// If the process never produced a run_end event, the actual failure
-			// is in runErr (binary missing, exec permission denied, etc.) —
-			// prefer it over the generic fallback so the real reason survives.
-			if detail == "" && runErr != nil {
-				detail = runErr.Error()
-			}
-			if detail == "" {
-				detail = "missing terminal run_end event"
-			}
-			outcome.Err = fmt.Errorf("zero exec failed: %s", detail)
-			return outcome
-		}
-
-		// Parse the captured trace. The trace is the attribution layer, so a
-		// missing/malformed file is not fatal — but it IS surfaced as a warning
-		// (via outcome.TraceIssue) so an incomplete measurement can't look valid.
-		if f, ferr := os.Open(tracePath); ferr != nil {
-			outcome.TraceIssue = fmt.Sprintf("trace open failed: %v", ferr)
-		} else {
-			tr, perr := trace.ReadNDJSON(f)
-			_ = f.Close()
-			if perr != nil {
-				outcome.TraceIssue = fmt.Sprintf("trace parse failed: %v", perr)
-			} else {
-				outcome.Trace = tr
-			}
-		}
-
-		// A nonzero agent exit already decided failure; don't run verification or
-		// mark the task passed.
-		if outcome.VerifyErr != "" {
-			return outcome
-		}
-		if len(task.VerificationCommand) > 0 {
-			if vOutcome := runVerification(ctx, task); !vOutcome.Passed {
-				outcome.VerifyErr = strings.TrimSpace(vOutcome.Detail)
-				return outcome
-			}
-			// A positive oracle (grep/test/build) passed: this is the only path
-			// that sets Passed. A task with no verificationCommand is latency-only
-			// (read-only nav/longproc/longctx/parallel): its exit 0 proves the turn
-			// ran, not that the answer was right, so it never reports Passed and the
-			// harness counts it in latencyOnlyTasks rather than any pass rate.
-			outcome.Passed = true
-		}
-		return outcome
-	}
-}
-
-// copyFixture copies the fixture directory at src into a fresh temp dir and
-// returns the copy's path. Used to give each benchmark invocation an isolated
-// workspace so mutating tasks (edit/fix/refactor) can't dirty the checked-in
-// fixture or a later iteration.
-func copyFixture(src string) (string, error) {
-	info, err := os.Stat(src)
-	if err != nil {
-		return "", err
-	}
-	if !info.IsDir() {
-		return "", fmt.Errorf("fixture %q is not a directory", src)
-	}
-	dst, err := os.MkdirTemp("", "zero-turn-fixture-*")
-	if err != nil {
-		return "", err
-	}
-	err = filepath.WalkDir(src, func(path string, d fs.DirEntry, werr error) error {
-		if werr != nil {
-			return werr
-		}
-		rel, rerr := filepath.Rel(src, path)
-		if rerr != nil {
-			return rerr
-		}
-		target := filepath.Join(dst, rel)
-		if d.IsDir() {
-			return os.MkdirAll(target, 0o755)
-		}
-		data, rerr := os.ReadFile(path)
-		if rerr != nil {
-			return rerr
-		}
-		return os.WriteFile(target, data, 0o644)
-	})
-	if err != nil {
-		os.RemoveAll(dst)
-		return "", err
-	}
-	return dst, nil
-}
-
-func buildTurnExecArgs(task BenchTask, rc RunContext, tracePath string, extraArgs []string) []string {
-	args := []string{"exec", "--output-format", "stream-json", "--trace", tracePath}
-	if model := strings.TrimSpace(rc.Model); model != "" {
-		args = append(args, "--model", model)
-	}
-	if mode := strings.TrimSpace(rc.Mode); mode != "" {
-		args = append(args, "--mode", mode)
-	}
-	if rc.SelfCorrect {
-		args = append(args, "--self-correct")
-	}
-	args = append(args, extraArgs...)
-	args = append(args, task.Prompt)
-	return args
-}
-
-// ResolveBinary locates the zero binary for a benchmark run: an explicit path
-// when provided, else a `zero` (or zero.exe) on PATH, else a binary built into
-// the repo root. Returns an error when none is found.
-func ResolveBinary(explicit string) (string, error) {
-	if v := strings.TrimSpace(explicit); v != "" {
-		if _, err := os.Stat(v); err != nil {
-			return "", fmt.Errorf("trace binary not found: %w", err)
-		}
-		return v, nil
-	}
-	if path, err := exec.LookPath("zero"); err == nil {
-		return path, nil
-	}
-	if path, err := exec.LookPath("zero.exe"); err == nil {
-		return path, nil
-	}
-	cwd, err := os.Getwd()
-	if err == nil {
-		for _, name := range []string{"zero", "zero.exe"} {
-			candidate := filepath.Join(cwd, name)
-			if _, err := os.Stat(candidate); err == nil {
-				return candidate, nil
-			}
-		}
-	}
-	return "", errors.New("zero binary not found; build it first or pass an explicit path")
-}

internal/perfbench/turn_bench_test.go

@@ -1,455 +0,0 @@
-package perfbench
-
-import (
-	"bytes"
-	"context"
-	"encoding/json"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-	"time"
-
-	"github.com/Gitlawb/zero/internal/trace"
-)
-
-// fakeTurnRunner returns a canned *trace.TurnTrace per task so the harness's
-// aggregation logic can be exercised without a model or a binary. Each task
-// yields one generation span (the dominant cost), one tool_execution span, and
-// the counters the totals aggregate.
-func fakeTurnRunner(canned map[string]*trace.TurnTrace) TurnRunner {
-	return func(ctx context.Context, task BenchTask, rc RunContext) TurnTaskOutcome {
-		tr, ok := canned[task.ID]
-		if !ok {
-			return TurnTaskOutcome{Err: errNoCanned}
-		}
-		wallMs := float64(tr.WallDuration().Microseconds()) / 1000
-		return TurnTaskOutcome{Passed: true, WallMs: wallMs, Trace: tr}
-	}
-}
-
-var errNoCanned = &cannedError{}
-
-type cannedError struct{}
-
-func (*cannedError) Error() string { return "no canned trace for task" }
-
-// cannedTrace builds a deterministic *trace.TurnTrace with the given spans and
-// counters. Spans are recorded as fixed durations (no real timing) so the
-// aggregation math is predictable in assertions.
-func cannedTrace(genMs, toolMs int, tokens int64) *trace.TurnTrace {
-	r := trace.NewRecorder("sess", "run-1", "test")
-	r.Start()
-	r.RecordSpan(trace.SpanGeneration, time.Duration(genMs)*time.Millisecond)
-	r.RecordSpan(trace.SpanToolExecution, time.Duration(toolMs)*time.Millisecond)
-	r.Counter(trace.CounterInputTokens, tokens)
-	r.Counter(trace.CounterOutputTokens, tokens/2)
-	r.Counter(trace.CounterModelRequests, 1)
-	r.Counter(trace.CounterToolCalls, 1)
-	r.StampFirstToken()
-	return r.Finish()
-}
-
-func TestRunTurnBenchAggregation(t *testing.T) {
-	set := TaskSet{
-		ID: "fake-suite",
-		Tasks: []BenchTask{
-			{ID: "t1", Class: "nav", Prompt: "p1"},                                             // latency-only
-			{ID: "t2", Class: "nav", Prompt: "p2"},                                             // latency-only
-			{ID: "t3", Class: "edit", Prompt: "p3", VerificationCommand: []string{"true"}},     // correctness
-			{ID: "t4", Class: "refactor", Prompt: "p4", VerificationCommand: []string{"true"}}, // build-only
-		},
-		BuildOnlyClasses: []string{"refactor"},
-	}
-	canned := map[string]*trace.TurnTrace{
-		"t1": cannedTrace(100, 10, 1000),
-		"t2": cannedTrace(300, 10, 1000),
-		"t3": cannedTrace(200, 50, 2000),
-		"t4": cannedTrace(150, 20, 1500),
-	}
-	cfg := TurnBenchConfig{
-		Model:      "fake-model",
-		Iterations: 1,
-		Runner:     fakeTurnRunner(canned),
-		Now:        func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) },
-	}
-	result, err := RunTurnBench(context.Background(), set, cfg)
-	if err != nil {
-		t.Fatalf("RunTurnBench: %v", err)
-	}
-	// 4 tasks attempted; the tier split is 2 latency-only, 1 correctness, 1 build.
-	if result.TasksAttempted != 4 {
-		t.Fatalf("attempted=%d, want 4", result.TasksAttempted)
-	}
-	if result.TasksVerified != 1 || result.TasksPassed != 1 {
-		t.Fatalf("correctness verified=%d passed=%d, want 1/1", result.TasksVerified, result.TasksPassed)
-	}
-	if result.LatencyOnlyTasks != 2 {
-		t.Fatalf("latencyOnly=%d, want 2", result.LatencyOnlyTasks)
-	}
-	if result.BuildCheckedTasks != 1 || result.BuildPassedTasks != 1 {
-		t.Fatalf("build checked=%d passed=%d, want 1/1", result.BuildCheckedTasks, result.BuildPassedTasks)
-	}
-	if result.CorrectnessPassRate != 1.0 {
-		t.Fatalf("correctnessPassRate=%v, want 1.0", result.CorrectnessPassRate)
-	}
-	if result.BuildPassRate != 1.0 {
-		t.Fatalf("buildPassRate=%v, want 1.0", result.BuildPassRate)
-	}
-	if len(result.CorrectnessClasses) != 1 || result.CorrectnessClasses[0] != "edit" {
-		t.Fatalf("correctnessClasses=%v, want [edit]", result.CorrectnessClasses)
-	}
-	if len(result.BuildOnlyClasses) != 1 || result.BuildOnlyClasses[0] != "refactor" {
-		t.Fatalf("buildOnlyClasses=%v, want [refactor]", result.BuildOnlyClasses)
-	}
-	if len(result.LatencyOnlyClasses) != 1 || result.LatencyOnlyClasses[0] != "nav" {
-		t.Fatalf("latencyOnlyClasses=%v, want [nav]", result.LatencyOnlyClasses)
-	}
-	if result.SchemaVersion != TurnSchemaVersion {
-		t.Fatalf("schemaVersion = %d, want %d", result.SchemaVersion, TurnSchemaVersion)
-	}
-	if result.Date != "2026-01-02T03:04:05Z" {
-		t.Fatalf("date = %q", result.Date)
-	}
-
-	// Per-span: generation appears in all four (100+300+200+150=750ms), tool in
-	// all four (10+10+50+20=90ms). Count must equal the number of tasks * iterations.
-	gen := result.PerSpan[trace.SpanGeneration]
-	if gen.Count != 4 {
-		t.Fatalf("generation count = %d, want 4", gen.Count)
-	}
-	if gen.TotalMs != 750 {
-		t.Fatalf("generation totalMs = %v, want 750", gen.TotalMs)
-	}
-	tool := result.PerSpan[trace.SpanToolExecution]
-	if tool.TotalMs != 90 {
-		t.Fatalf("tool totalMs = %v, want 90", tool.TotalMs)
-	}
-
-	// Top latency: generation (750) ranks above tool (90). Exactly two spans
-	// here, so both appear and generation is first.
-	if len(result.TopLatency) != 2 || result.TopLatency[0].Span != trace.SpanGeneration {
-		t.Fatalf("topLatency = %+v", result.TopLatency)
-	}
-	if result.TopLatency[0].Share <= result.TopLatency[1].Share {
-		t.Fatalf("top latency not ranked by share: %+v", result.TopLatency)
-	}
-
-	// Totals: 4 model requests, 4 tool calls, input tokens 1000+1000+2000+1500=5500.
-	if result.Totals.ModelRequests != 4 {
-		t.Fatalf("modelRequests = %d, want 4", result.Totals.ModelRequests)
-	}
-	if result.Totals.ToolCalls != 4 {
-		t.Fatalf("toolCalls = %d, want 4", result.Totals.ToolCalls)
-	}
-	if result.Totals.InputTokens != 5500 {
-		t.Fatalf("inputTokens = %d, want 5500", result.Totals.InputTokens)
-	}
-	if result.Totals.OutputTokens != 2750 {
-		t.Fatalf("outputTokens = %d, want 2750", result.Totals.OutputTokens)
-	}
-
-	// Per-class tier roll-up: nav is latency-only (0 verified, 2 latency-only),
-	// edit is correctness (1/1 verified passed), refactor is build (1/1 passed).
-	nav := result.PerClass["nav"]
-	if nav.Tasks != 2 || nav.Verified != 0 || nav.Passed != 0 || nav.LatencyOnly != 2 {
-		t.Fatalf("nav class = %+v", nav)
-	}
-	edit := result.PerClass["edit"]
-	if edit.Tasks != 1 || edit.Verified != 1 || edit.Passed != 1 || edit.LatencyOnly != 0 {
-		t.Fatalf("edit class = %+v", edit)
-	}
-	refactor := result.PerClass["refactor"]
-	if refactor.Tasks != 1 || refactor.Verified != 1 || refactor.Passed != 1 || refactor.LatencyOnly != 0 {
-		t.Fatalf("refactor class = %+v", refactor)
-	}
-}
-
-// TestRunTurnBenchLatencyOnlyNeverPassed asserts the honesty gate: a task with
-// no verificationCommand reports Passed=true from the (stub) runner, yet the
-// harness counts it ONLY in latencyOnlyTasks — never in tasksPassed or any pass
-// rate — so an exit-0 read-only run cannot inflate a correctness number.
-func TestRunTurnBenchLatencyOnlyNeverPassed(t *testing.T) {
-	set := TaskSet{
-		ID: "lo-suite",
-		Tasks: []BenchTask{
-			{ID: "n1", Class: "nav", Prompt: "p"}, // no oracle — runner still says Passed=true
-		},
-	}
-	cfg := TurnBenchConfig{
-		Model:  "fake-model",
-		Runner: fakeTurnRunner(map[string]*trace.TurnTrace{"n1": cannedTrace(50, 5, 100)}),
-		Now:    func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) },
-	}
-	result, err := RunTurnBench(context.Background(), set, cfg)
-	if err != nil {
-		t.Fatalf("RunTurnBench: %v", err)
-	}
-	if result.TasksPassed != 0 || result.TasksVerified != 0 {
-		t.Fatalf("latency-only leaked into pass: passed=%d verified=%d, want 0/0",
-			result.TasksPassed, result.TasksVerified)
-	}
-	if result.CorrectnessPassRate != 0 || result.BuildPassRate != 0 {
-		t.Fatalf("pass rates should be 0 with no oracle tasks: c=%v b=%v",
-			result.CorrectnessPassRate, result.BuildPassRate)
-	}
-	if result.LatencyOnlyTasks != 1 {
-		t.Fatalf("latencyOnlyTasks=%d, want 1", result.LatencyOnlyTasks)
-	}
-	if result.PerClass["nav"].Passed != 0 || result.PerClass["nav"].LatencyOnly != 1 {
-		t.Fatalf("nav class = %+v, want passed=0 latencyOnly=1", result.PerClass["nav"])
-	}
-}
-
-func TestRunTurnBenchIterationsAggregates(t *testing.T) {
-	set := TaskSet{
-		ID:    "iter-suite",
-		Tasks: []BenchTask{{ID: "t1", Class: "nav", Prompt: "p1"}},
-	}
-	canned := map[string]*trace.TurnTrace{"t1": cannedTrace(100, 10, 500)}
-	cfg := TurnBenchConfig{
-		Model:      "fake-model",
-		Iterations: 3,
-		Runner:     fakeTurnRunner(canned),
-		Now:        func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) },
-	}
-	result, err := RunTurnBench(context.Background(), set, cfg)
-	if err != nil {
-		t.Fatalf("RunTurnBench: %v", err)
-	}
-	if result.PerSpan[trace.SpanGeneration].Count != 3 {
-		t.Fatalf("generation count = %d, want 3 (one per iteration)", result.PerSpan[trace.SpanGeneration].Count)
-	}
-	if result.PerSpan[trace.SpanGeneration].TotalMs != 300 {
-		t.Fatalf("generation totalMs = %v, want 300", result.PerSpan[trace.SpanGeneration].TotalMs)
-	}
-}
-
-func TestRunTurnBenchRequiresModelAndRunner(t *testing.T) {
-	set := TaskSet{ID: "s", Tasks: []BenchTask{{ID: "t", Prompt: "p"}}}
-	if _, err := RunTurnBench(context.Background(), set, TurnBenchConfig{Runner: fakeTurnRunner(nil)}); err == nil {
-		t.Fatal("expected error for missing model")
-	}
-	if _, err := RunTurnBench(context.Background(), set, TurnBenchConfig{Model: "m"}); err == nil {
-		t.Fatal("expected error for missing runner")
-	}
-	if _, err := RunTurnBench(context.Background(), TaskSet{ID: "empty"}, TurnBenchConfig{Model: "m", Runner: fakeTurnRunner(nil)}); err == nil {
-		t.Fatal("expected error for empty task set")
-	}
-}
-
-func TestTopLatencySourcesRanksByTotalAndCapsTopN(t *testing.T) {
-	perSpan := map[string]SpanStats{
-		"a": {TotalMs: 100},
-		"b": {TotalMs: 500},
-		"c": {TotalMs: 300},
-		"d": {TotalMs: 50},
-	}
-	top := topLatencySources(perSpan, 3)
-	if len(top) != 3 {
-		t.Fatalf("len = %d, want 3", len(top))
-	}
-	wantOrder := []string{"b", "c", "a"}
-	for i, w := range wantOrder {
-		if top[i].Span != w {
-			t.Fatalf("top[%d] = %q, want %q", i, top[i].Span, w)
-		}
-	}
-	// Shares sum to 1 across all four (100+500+300+50=950); the top-3 retain
-	// their global share (not renormalized to the top-3).
-	// Share is rounded to 2 decimals by RoundMetric (500/950 -> 0.53), so compare
-	// against the rounded value with a small tolerance.
-	if got, want := top[0].Share, RoundMetric(500.0/950.0); !approxEqual(got, want, 0.001) {
-		t.Fatalf("top[0] share = %v, want %v", got, want)
-	}
-}
-
-func TestWriteTurnBenchJSONRoundTrip(t *testing.T) {
-	set := TaskSet{ID: "json-suite", Tasks: []BenchTask{
-		{ID: "t1", Class: "edit", Prompt: "p1", VerificationCommand: []string{"true"}},
-	}}
-	canned := map[string]*trace.TurnTrace{"t1": cannedTrace(150, 20, 800)}
-	result, err := RunTurnBench(context.Background(), set, TurnBenchConfig{
-		Model:  "fake-model",
-		Runner: fakeTurnRunner(canned),
-		Now:    func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) },
-	})
-	if err != nil {
-		t.Fatalf("RunTurnBench: %v", err)
-	}
-	var buf bytes.Buffer
-	if err := WriteTurnBenchJSON(&buf, result); err != nil {
-		t.Fatalf("WriteTurnBenchJSON: %v", err)
-	}
-	var decoded TurnBenchResult
-	if err := json.Unmarshal(buf.Bytes(), &decoded); err != nil {
-		t.Fatalf("unmarshal: %v\n%s", err, buf.String())
-	}
-	if decoded.SchemaVersion != TurnSchemaVersion {
-		t.Fatalf("schemaVersion = %d, want %d", decoded.SchemaVersion, TurnSchemaVersion)
-	}
-	if decoded.TasksVerified != 1 || decoded.TasksPassed != 1 || decoded.LatencyOnlyTasks != 0 {
-		t.Fatalf("decoded tier counts = verified=%d passed=%d latency=%d, want 1/1/0",
-			decoded.TasksVerified, decoded.TasksPassed, decoded.LatencyOnlyTasks)
-	}
-	if decoded.CorrectnessPassRate != 1.0 {
-		t.Fatalf("decoded correctnessPassRate = %v, want 1.0", decoded.CorrectnessPassRate)
-	}
-	if decoded.PerSpan[trace.SpanGeneration].TotalMs != 150 {
-		t.Fatalf("decoded generation totalMs = %v, want 150", decoded.PerSpan[trace.SpanGeneration].TotalMs)
-	}
-}
-
-func TestFormatTurnBenchSummaryNamesTopSources(t *testing.T) {
-	set := TaskSet{ID: "fmt-suite", Tasks: []BenchTask{{ID: "t1", Class: "nav", Prompt: "p1"}}}
-	canned := map[string]*trace.TurnTrace{"t1": cannedTrace(150, 20, 800)}
-	result, err := RunTurnBench(context.Background(), set, TurnBenchConfig{
-		Model:  "fake-model",
-		Runner: fakeTurnRunner(canned),
-		Now:    func() time.Time { return time.Date(2026, 1, 2, 3, 4, 5, 0, time.UTC) },
-	})
-	if err != nil {
-		t.Fatalf("RunTurnBench: %v", err)
-	}
-	summary := FormatTurnBenchSummary(result)
-	if !strings.Contains(summary, "top latency sources") {
-		t.Fatalf("summary missing top-latency header:\n%s", summary)
-	}
-	if !strings.Contains(summary, trace.SpanGeneration) {
-		t.Fatalf("summary missing top span name %q:\n%s", trace.SpanGeneration, summary)
-	}
-}
-
-func TestLoadBaselineManifest(t *testing.T) {
-	path := filepath.Join("manifests", "baseline.json")
-	set, err := LoadTaskSet(path)
-	if err != nil {
-		t.Fatalf("LoadTaskSet: %v", err)
-	}
-	if set.ID == "" {
-		t.Fatal("manifest has no id")
-	}
-	// The baseline must clear the "do not proceed until ≥30 tasks" gate.
-	if len(set.Tasks) < 30 {
-		t.Fatalf("baseline has %d tasks, want >= 30", len(set.Tasks))
-	}
-	// The six required classes must all be present and non-empty.
-	wantClasses := map[string]bool{
-		"nav": false, "edit": false, "fix": false,
-		"refactor": false, "longproc": false, "longctx": false, "parallel": false,
-	}
-	counts := map[string]int{}
-	for _, task := range set.Tasks {
-		class := strings.TrimSpace(task.Class)
-		if class == "" {
-			t.Fatalf("task %q has no class", task.ID)
-		}
-		if _, ok := wantClasses[class]; !ok {
-			t.Fatalf("unexpected class %q on task %q", class, task.ID)
-		}
-		wantClasses[class] = true
-		counts[class]++
-	}
-	for class, present := range wantClasses {
-		if !present {
-			t.Fatalf("manifest missing required class %q", class)
-		}
-		if counts[class] == 0 {
-			t.Fatalf("class %q has zero tasks", class)
-		}
-	}
-	// Every task must have a prompt and a workspace fixture pointing under testdata,
-	// and that fixture must actually exist on disk — a manifest referencing a
-	// missing fixture would make every task in its class error out at run time, so
-	// catch it at load time instead.
-	seen := map[string]bool{}
-	for _, task := range set.Tasks {
-		if strings.TrimSpace(task.Prompt) == "" {
-			t.Fatalf("task %q has empty prompt", task.ID)
-		}
-		if strings.TrimSpace(task.WorkspaceFixture) == "" {
-			t.Fatalf("task %q has no workspace fixture", task.ID)
-		}
-		if !strings.Contains(task.WorkspaceFixture, "testdata") {
-			t.Fatalf("task %q fixture %q not under testdata", task.ID, task.WorkspaceFixture)
-		}
-		if seen[task.WorkspaceFixture] {
-			continue
-		}
-		seen[task.WorkspaceFixture] = true
-		info, err := os.Stat(task.WorkspaceFixture)
-		if err != nil {
-			t.Fatalf("task %q fixture %q does not exist: %v", task.ID, task.WorkspaceFixture, err)
-		}
-		if !info.IsDir() {
-			t.Fatalf("task %q fixture %q is not a directory", task.ID, task.WorkspaceFixture)
-		}
-	}
-}
-
-func approxEqual(a, b, tol float64) bool {
-	d := a - b
-	if d < 0 {
-		d = -d
-	}
-	return d < tol
-}
-
-// TestCopyFixtureIsolatesSourceFromMutation asserts the property that lets a
-// mutating task run twice (or across iterations) without poisoning the next
-// sample: the runner operates on a per-invocation copy, so mutating the copy
-// leaves the checked-in source fixture byte-identical. This is verified by
-// reading the code today; a test makes it durable against future refactors of
-// the runner's isolation path.
-func TestCopyFixtureIsolatesSourceFromMutation(t *testing.T) {
-	src, err := os.MkdirTemp("", "zero-fixture-src-*")
-	if err != nil {
-		t.Fatalf("mkdtemp src: %v", err)
-	}
-	defer os.RemoveAll(src)
-	orig := "package main\n\nfunc main() {}\n"
-	if err := os.WriteFile(filepath.Join(src, "main.go"), []byte(orig), 0o644); err != nil {
-		t.Fatalf("write fixture: %v", err)
-	}
-	if err := os.MkdirAll(filepath.Join(src, "sub"), 0o755); err != nil {
-		t.Fatalf("mkdir sub: %v", err)
-	}
-	if err := os.WriteFile(filepath.Join(src, "sub", "a.go"), []byte("package sub\n"), 0o644); err != nil {
-		t.Fatalf("write sub: %v", err)
-	}
-
-	// Snapshot the source tree before any mutation of the copy.
-	wantMain, err := os.ReadFile(filepath.Join(src, "main.go"))
-	if err != nil {
-		t.Fatalf("read source main.go: %v", err)
-	}
-
-	copyDir, err := copyFixture(src)
-	if err != nil {
-		t.Fatalf("copyFixture: %v", err)
-	}
-	defer os.RemoveAll(copyDir)
-
-	// The copy must be a real copy, not a symlink/alias of the source, and
-	// mutating it must not touch the source.
-	if copyDir == src {
-		t.Fatalf("copyFixture returned the source dir, not a copy: %s", copyDir)
-	}
-	if err := os.WriteFile(filepath.Join(copyDir, "main.go"), []byte("package main\n\n// mutated\nfunc main() {}\n"), 0o644); err != nil {
-		t.Fatalf("mutate copy: %v", err)
-	}
-	if err := os.WriteFile(filepath.Join(copyDir, "new.go"), []byte("package main\n"), 0o644); err != nil {
-		t.Fatalf("add file to copy: %v", err)
-	}
-
-	gotMain, err := os.ReadFile(filepath.Join(src, "main.go"))
-	if err != nil {
-		t.Fatalf("re-read source main.go: %v", err)
-	}
-	if string(gotMain) != string(wantMain) {
-		t.Fatalf("source fixture mutated by copy: got %q, want %q", gotMain, wantMain)
-	}
-	if _, err := os.Stat(filepath.Join(src, "new.go")); !os.IsNotExist(err) {
-		t.Fatalf("file added to copy appeared in source: %v", err)
-	}
-}

internal/plugins/activate.go

@@ -271,55 +271,72 @@ func skillSearchRoot(path string) string {
 // recognize a manifest path that points at the file rather than the directory.
 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
-// 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
-// yields no skills rather than failing the merge.
+// MergedSkills loads the default skills directory plus the supplied plugin skill
+// roots and returns one merged, name-deduplicated list (Content stripped, like
+// skills.List) 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 a same-named plugin skill.
+// A bad root simply yields no skills rather than failing the merge.
 func MergedSkills(defaultDir string, pluginRoots []string) ([]skills.Skill, []skills.DuplicateName) {
 	return mergeSkills(defaultDir, pluginRoots, false)
 }
 
-// mergeSkills merges the primary skills dir, optional ~/.agents/skills, and
-// plugin roots into one sorted, name-deduplicated list via skills.LoadFromRoots /
-// ListFromRoots. keepContent retains each skill's body versus stripping it.
-// Roots are considered in order with the default dir first, so an earlier root
-// wins a name clash; collisions are recorded rather than crashing.
+// mergeSkills merges the default skills dir and plugin roots into one sorted,
+// name-deduplicated list. keepContent retains each skill's body (skills.Load)
+// versus stripping it (skills.List). Roots are considered in order with the
+// default dir first, so an earlier root wins a name clash; collisions are recorded
+// rather than crashing, preserving the skills package's Duplicates behaviour.
 func mergeSkills(defaultDir string, pluginRoots []string, keepContent bool) ([]skills.Skill, []skills.DuplicateName) {
-	// Keep defaultDir injectable for tests rather than always calling DefaultDir.
-	roots := skills.GlobalRoots(defaultDir)
-	// GlobalRoots already includes AgentsDir; append plugin roots only.
-	for _, root := range pluginRoots {
-		if root = strings.TrimSpace(root); root != "" {
-			roots = append(roots, root)
+	merged := []skills.Skill{}
+	dups := []skills.DuplicateName{}
+	byName := map[string]skills.Skill{}
+
+	roots := append([]string{defaultDir}, pluginRoots...)
+	for _, root := range roots {
+		if strings.TrimSpace(root) == "" {
+			continue
+		}
+		var loaded []skills.Skill
+		var err error
+		if keepContent {
+			loaded, err = skills.Load(root)
+		} else {
+			loaded, err = skills.List(root)
+		}
+		if err != nil {
+			continue
+		}
+		for _, skill := range loaded {
+			if winner, clash := byName[skill.Name]; clash {
+				dups = append(dups, skills.DuplicateName{Name: skill.Name, Winner: winner.Path, Loser: skill.Path})
+				continue
+			}
+			byName[skill.Name] = skill
+			merged = append(merged, skill)
 		}
 	}
-	if keepContent {
-		merged, dups, _ := skills.LoadFromRoots(roots)
-		return merged, dups
-	}
-	merged, dups, _ := skills.ListFromRoots(roots)
+
+	sort.Slice(merged, func(left int, right int) bool {
+		return merged[left].Name < merged[right].Name
+	})
 	return merged, dups
 }
 
 // skillTool is a drop-in replacement for the core skill tool that resolves a
-// named skill across the default skills directory, the shared ~/.agents/skills
-// root, and plugin skill roots, so agents- and plugin-declared skills surface in
-// the agent's skill list. It keeps the core tool's name, schema, and
-// read-only/allow safety so registering it simply overlays multi-root discovery
-// onto the existing surface.
+// named skill across the default skills directory PLUS the plugin skill roots, so
+// plugin-declared skills surface in the agent's skill list. It keeps the core
+// tool's name, schema, and read-only/allow safety so registering it simply
+// overlays plugin skills onto the existing surface.
 type skillTool struct {
 	defaultDir  string
 	pluginRoots []string
 }
 
-// NewSkillTool builds the multi-root skill tool. defaultDir is the standard
+// NewSkillTool builds the plugin-aware skill tool. defaultDir is the standard
 // skills directory (skills.DefaultDir); pluginRoots are the plugin skill search
-// roots from an ActivationResult. The returned tool merges primary + agents +
-// plugins, deterministically deduplicating by name (default dir wins a clash)
-// and listing all available skills when an unknown name is requested.
+// roots from an ActivationResult. The returned tool merges both, deterministically
+// deduplicating by name (default dir wins a clash) and listing all available
+// skills when an unknown name is requested.
 func NewSkillTool(defaultDir string, pluginRoots []string) tools.Tool {
 	return skillTool{defaultDir: defaultDir, pluginRoots: append([]string{}, pluginRoots...)}
 }
@@ -328,7 +345,7 @@ func (tool skillTool) Name() string { return "skill" }
 
 func (tool skillTool) Description() string {
 	return "Load a named Zero skill and return its instructions as the tool output. " +
-		"Skills are reusable, on-demand instruction sets (including shared ~/.agents/skills and any contributed by plugins). " +
+		"Skills are reusable, on-demand instruction sets (including any contributed by plugins). " +
 		"Call this when a relevant skill exists; an unknown name returns the list of available skills."
 }
 

internal/plugins/activate_test.go

@@ -350,7 +350,6 @@ func TestActivateResolvesManifestRelativeSkillRoot(t *testing.T) {
 }
 
 func TestNewSkillToolSurfacesPluginSkillInList(t *testing.T) {
-	isolateAgentsHome(t)
 	defaultDir := t.TempDir()
 	writeTestSkill(t, defaultDir, "core-skill", "core body")
 	pluginRoot := t.TempDir()
@@ -385,7 +384,6 @@ func TestNewSkillToolSurfacesPluginSkillInList(t *testing.T) {
 }
 
 func TestMergeSkillRootsListsPluginSkillInAgentList(t *testing.T) {
-	isolateAgentsHome(t)
 	defaultDir := t.TempDir()
 	writeTestSkill(t, defaultDir, "core-skill", "core")
 
@@ -406,9 +404,6 @@ func TestMergeSkillRootsListsPluginSkillInAgentList(t *testing.T) {
 }
 
 func TestMergedSkillsRecordsDuplicatesWithoutCrashing(t *testing.T) {
-	// Isolate AgentsDir so a host ~/.agents/skills cannot add extra collisions.
-	isolateAgentsHome(t)
-
 	defaultDir := t.TempDir()
 	writeTestSkill(t, defaultDir, "shared", "default copy")
 
@@ -431,90 +426,6 @@ func TestMergedSkillsRecordsDuplicatesWithoutCrashing(t *testing.T) {
 	}
 }
 
-// isolateAgentsHome points HOME/USERPROFILE at an empty temp home so AgentsDir
-// does not pick up the developer's real ~/.agents/skills during unit tests.
-func isolateAgentsHome(t *testing.T) {
-	t.Helper()
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-}
-
-func TestMergedSkillsIncludesAgentsWithoutPlugins(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	writeTestSkill(t, agents, "agents-skill", "from agents")
-
-	defaultDir := t.TempDir()
-	listed, dups := MergedSkills(defaultDir, nil)
-	if len(dups) != 0 {
-		t.Fatalf("unexpected dups: %#v", dups)
-	}
-	if len(listed) != 1 || listed[0].Name != "agents-skill" {
-		t.Fatalf("agents skill should surface with no plugins, got %#v", listed)
-	}
-}
-
-func TestMergedSkillsPriorityPrimaryAgentsPlugins(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-
-	primary := t.TempDir()
-	pluginRoot := t.TempDir()
-	writeTestSkill(t, primary, "shared", "primary body")
-	writeTestSkill(t, agents, "shared", "agents body")
-	writeTestSkill(t, agents, "agents-plugin", "agents body")
-	writeTestSkill(t, pluginRoot, "shared", "plugin body")
-	writeTestSkill(t, pluginRoot, "agents-plugin", "plugin body")
-	writeTestSkill(t, pluginRoot, "plugin-only", "plugin only")
-
-	listed, dups := MergedSkills(primary, []string{pluginRoot})
-	byName := map[string]string{}
-	for _, skill := range listed {
-		// Content is stripped by MergedSkills; use path origin instead.
-		byName[skill.Name] = skill.Path
-	}
-	if !strings.Contains(byName["shared"], primary) {
-		t.Fatalf("primary should win shared, path=%q", byName["shared"])
-	}
-	if !strings.Contains(byName["agents-plugin"], agents) {
-		t.Fatalf("agents should win over plugin for agents-plugin, path=%q", byName["agents-plugin"])
-	}
-	if !strings.Contains(byName["plugin-only"], pluginRoot) {
-		t.Fatalf("plugin-only missing, path=%q", byName["plugin-only"])
-	}
-	if len(dups) < 2 {
-		t.Fatalf("expected cross-layer dups, got %#v", dups)
-	}
-}
-
-func TestNewSkillToolLoadsAgentsSkillWithoutPluginRoots(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	writeTestSkill(t, agents, "agents-skill", "agents body")
-
-	tool := NewSkillTool(t.TempDir(), nil)
-	got := tool.Run(context.Background(), map[string]any{"name": "agents-skill"})
-	if got.Status != tools.StatusOK || !strings.Contains(got.Output, "agents body") {
-		t.Fatalf("expected agents skill body with zero plugin roots, got %q (%s)", got.Status, got.Output)
-	}
-}
-
 func TestActivateSkipsMalformedPluginAndContinues(t *testing.T) {
 	registry := tools.NewRegistry()
 	good := toolPlugin(t.TempDir(), ToolExtension{Name: "good", Command: "good", Permission: PermissionPrompt})

internal/providercatalog/catalog.go

@@ -34,9 +34,6 @@ const (
 
 var ErrUnknownProvider = errors.New("unknown provider")
 
-// AIMLAPIID is the provider catalog identifier for the AI/ML API preset.
-const AIMLAPIID = "aimlapi"
-
 type Descriptor struct {
 	ID                  string
 	Name                string
@@ -110,7 +107,6 @@ var descriptors = []Descriptor{
 	localOpenAI("ollama", "Ollama Local", "http://localhost:11434/v1", "llama3.1", "ollama local"),
 	localOpenAI("lmstudio", "LM Studio", "http://localhost:1234/v1", "local-model", "lm-studio", "lm studio"),
 	oauthProvider(openAICompat("openrouter", "OpenRouter", "https://openrouter.ai/api/v1", "openai/gpt-4.1", []string{"OPENROUTER_API_KEY"}), true, false),
-	openAICompat(AIMLAPIID, "AI/ML API", "https://api.aimlapi.com/v1", "openai/gpt-5-chat", []string{"AIMLAPI_API_KEY"}, "aiml api", "ai/ml api"),
 	// Hugging Face Inference Providers — OpenAI-compatible router at
 	// https://router.huggingface.co/v1 exposes hundreds of OSS models. OAuth
 	// requires a one-time app registration at huggingface.co/settings/applications/new

internal/providercatalog/catalog_test.go

@@ -16,7 +16,6 @@ var expectedCatalogIDs = []string{
 	"ollama",
 	"lmstudio",
 	"openrouter",
-	"aimlapi",
 	"huggingface",
 	"chatgpt",
 	"groq",
@@ -104,28 +103,6 @@ func TestRecommendedProviderEndpoint(t *testing.T) {
 	}
 }
 
-func TestAIMLAPIDescriptor(t *testing.T) {
-	descriptor, err := Require("aimlapi")
-	if err != nil {
-		t.Fatalf("Require(aimlapi) error = %v", err)
-	}
-	if descriptor.Name != "AI/ML API" {
-		t.Fatalf("Name = %q, want AI/ML API", descriptor.Name)
-	}
-	if descriptor.DefaultBaseURL != "https://api.aimlapi.com/v1" {
-		t.Fatalf("DefaultBaseURL = %q, want AI/ML API endpoint", descriptor.DefaultBaseURL)
-	}
-	if descriptor.DefaultModel != "openai/gpt-5-chat" {
-		t.Fatalf("DefaultModel = %q, want openai/gpt-5-chat", descriptor.DefaultModel)
-	}
-	if descriptor.Transport != TransportOpenAICompatible {
-		t.Fatalf("Transport = %q, want %q", descriptor.Transport, TransportOpenAICompatible)
-	}
-	if !reflect.DeepEqual(descriptor.AuthEnvVars, []string{"AIMLAPI_API_KEY"}) {
-		t.Fatalf("AuthEnvVars = %#v, want AIMLAPI_API_KEY", descriptor.AuthEnvVars)
-	}
-}
-
 func TestLongCatDescriptor(t *testing.T) {
 	descriptor, err := Require("longcat")
 	if err != nil {
@@ -316,7 +293,7 @@ func TestListByTransportPreservesCatalogOrder(t *testing.T) {
 		TransportBedrock:         {"bedrock"},
 		TransportVertex:          {"vertex"},
 		TransportAnthropicCompat: {"minimax", "minimaxi-cn", "opencode-go-anthropic-compatible", "custom-anthropic-compatible"},
-		TransportOpenAICompat:    {"gitlawb-opengateway", "ollama-cloud", "ollama", "lmstudio", "openrouter", "aimlapi", "huggingface", "chatgpt", "groq", "deepseek", "together", "dashscope", "moonshot", "longcat", "nvidia-nim", "mistral", "github", "xai", "venice", "xiaomi-mimo", "bankr", "zai", "zai-cn", "kilocode", "opencode", "opencode-go", "atomic-chat", "chatgpt-proxy", "custom-openai-compatible"},
+		TransportOpenAICompat:    {"gitlawb-opengateway", "ollama-cloud", "ollama", "lmstudio", "openrouter", "huggingface", "chatgpt", "groq", "deepseek", "together", "dashscope", "moonshot", "longcat", "nvidia-nim", "mistral", "github", "xai", "venice", "xiaomi-mimo", "bankr", "zai", "zai-cn", "kilocode", "opencode", "opencode-go", "atomic-chat", "chatgpt-proxy", "custom-openai-compatible"},
 	}
 
 	for transport, wantIDs := range cases {

internal/providermodeldiscovery/discovery.go

@@ -195,7 +195,7 @@ func discoverOpenAIModels(ctx context.Context, profile config.ProviderProfile, o
 		AuthHeader:        profile.AuthHeader,
 		AuthScheme:        profile.AuthScheme,
 		AuthHeaderValue:   profile.AuthHeaderValue,
-		CustomHeaders:     providerio.CopyHeaders(profile.CustomHeaders),
+		CustomHeaders:     profile.CustomHeaders,
 	}, nil)
 }
 

internal/providermodeldiscovery/discovery_test.go

@@ -52,44 +52,6 @@ func TestDiscoverOpenAICompatibleModelsFetchesModelsEndpoint(t *testing.T) {
 	}
 }
 
-func TestDiscoverAIMLAPIModelsSendsAuthAndCustomHeadersWithoutAttribution(t *testing.T) {
-	server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
-		for header, want := range map[string]string{
-			"Authorization": "Bearer test-key",
-			"X-Trace":       "test",
-		} {
-			if got := r.Header.Get(header); got != want {
-				t.Errorf("%s = %q, want %q", header, got, want)
-			}
-		}
-		// No first-party referral/attribution headers are injected for catalog
-		// presets; aimlapi rides through CopyHeaders like every other provider.
-		for _, header := range []string{
-			"X-AIMLAPI-Partner-ID",
-			"X-AIMLAPI-Integration-Repo",
-			"X-AIMLAPI-Integration-Version",
-		} {
-			if got := r.Header.Get(header); got != "" {
-				t.Errorf("%s = %q, want no attribution header", header, got)
-			}
-		}
-		w.Header().Set("Content-Type", "application/json")
-		_, _ = w.Write([]byte(`{"data":[{"id":"openai/gpt-5-chat"}]}`))
-	}))
-	defer server.Close()
-
-	_, err := Discover(context.Background(), config.ProviderProfile{
-		CatalogID:     "aimlapi",
-		ProviderKind:  config.ProviderKindOpenAICompatible,
-		BaseURL:       server.URL + "/v1",
-		APIKey:        "test-key",
-		CustomHeaders: map[string]string{"X-Trace": "test"},
-	}, Options{HTTPClient: server.Client()})
-	if err != nil {
-		t.Fatalf("Discover returned error: %v", err)
-	}
-}
-
 func TestDiscoverOpenAICompatibleModelsHonorsAuthHeaderValue(t *testing.T) {
 	// A profile can authenticate via a raw auth-header value instead of APIKey;
 	// discovery must send it rather than probe unauthenticated.

internal/providers/factory.go

@@ -67,7 +67,7 @@ func New(profile config.ProviderProfile, options Options) (zeroruntime.Provider,
 			AuthHeader:            profile.AuthHeader,
 			AuthScheme:            profile.AuthScheme,
 			AuthHeaderValue:       profile.AuthHeaderValue,
-			CustomHeaders:         providerio.CopyHeaders(profile.CustomHeaders),
+			CustomHeaders:         profile.CustomHeaders,
 			OAuthResolver:         options.OAuthResolver,
 			MaxTokens:             resolved.maxOutputTokens,
 			HTTPClient:            options.HTTPClient,
@@ -83,7 +83,7 @@ func New(profile config.ProviderProfile, options Options) (zeroruntime.Provider,
 			AuthHeader:      profile.AuthHeader,
 			AuthScheme:      profile.AuthScheme,
 			AuthHeaderValue: profile.AuthHeaderValue,
-			CustomHeaders:   providerio.CopyHeaders(profile.CustomHeaders),
+			CustomHeaders:   profile.CustomHeaders,
 			OAuthResolver:   options.OAuthResolver,
 			MaxTokens:       resolved.maxOutputTokens,
 			HTTPClient:      options.HTTPClient,
@@ -97,7 +97,7 @@ func New(profile config.ProviderProfile, options Options) (zeroruntime.Provider,
 			AuthHeader:      profile.AuthHeader,
 			AuthScheme:      profile.AuthScheme,
 			AuthHeaderValue: profile.AuthHeaderValue,
-			CustomHeaders:   providerio.CopyHeaders(profile.CustomHeaders),
+			CustomHeaders:   profile.CustomHeaders,
 			OAuthResolver:   options.OAuthResolver,
 			MaxTokens:       resolved.maxOutputTokens,
 			HTTPClient:      options.HTTPClient,

internal/providers/factory_test.go

@@ -220,55 +220,6 @@ func TestNewThreadsCustomProviderHeaders(t *testing.T) {
 	}
 }
 
-func TestNewAIMLAPIProviderSendsEndpointAndAuthWithoutAttribution(t *testing.T) {
-	transport := &captureTransport{responseBody: "data: [DONE]\n\n"}
-	provider, err := New(config.ProviderProfile{
-		Name:          "aimlapi",
-		CatalogID:     "aimlapi",
-		ProviderKind:  config.ProviderKindOpenAICompatible,
-		APIKey:        "aimlapi-test-key",
-		Model:         "openai/gpt-5-chat",
-		CustomHeaders: map[string]string{"X-Trace": "test"},
-	}, Options{HTTPClient: &http.Client{Transport: transport}})
-	if err != nil {
-		t.Fatalf("New() error = %v", err)
-	}
-	stream, err := provider.StreamCompletion(context.Background(), zeroruntime.CompletionRequest{
-		Messages: []zeroruntime.Message{{Role: zeroruntime.MessageRoleUser, Content: "hello"}},
-	})
-	if err != nil {
-		t.Fatalf("StreamCompletion() error = %v", err)
-	}
-	for range stream {
-	}
-
-	if transport.request == nil {
-		t.Fatal("HTTP client was not used")
-	}
-	if got := transport.request.URL.String(); got != "https://api.aimlapi.com/v1/chat/completions" {
-		t.Fatalf("request URL = %q, want AI/ML API endpoint", got)
-	}
-	for header, want := range map[string]string{
-		"Authorization": "Bearer aimlapi-test-key",
-		"X-Trace":       "test",
-	} {
-		if got := transport.request.Header.Get(header); got != want {
-			t.Fatalf("%s = %q, want %q", header, got, want)
-		}
-	}
-	// No first-party referral/attribution headers are injected for catalog
-	// presets; aimlapi rides through CopyHeaders like every other provider.
-	for _, header := range []string{
-		"X-AIMLAPI-Partner-ID",
-		"X-AIMLAPI-Integration-Repo",
-		"X-AIMLAPI-Integration-Version",
-	} {
-		if got := transport.request.Header.Get(header); got != "" {
-			t.Fatalf("%s = %q, want no attribution header", header, got)
-		}
-	}
-}
-
 func TestNewSupportsOpenAIProviderKind(t *testing.T) {
 	provider, err := New(config.ProviderProfile{
 		Name:         "openai",

internal/providers/providerio/auth.go

@@ -4,8 +4,6 @@ import (
 	"context"
 	"net/http"
 	"strings"
-
-	"github.com/Gitlawb/zero/internal/trace"
 )
 
 // TokenResolver yields a fresh OAuth credential for one request, or ok=false to
@@ -44,12 +42,7 @@ func SendWithAuthRetry(
 		// request (leaking the path/body) before we return the error.
 		headers := base
 		if resolver != nil {
-			// ProviderQueue captures pre-send auth-wait (OAuth token resolve). No
-			// send-side semaphore exists today; a future request queue would also
-			// accumulate here. nil recorder (untraced) is a no-op.
-			queueSpan := trace.FromContext(ctx).Span(trace.SpanProviderQueue)
 			header, value, ok, rerr := resolver(ctx, forceRefresh)
-			queueSpan.End()
 			if rerr != nil {
 				return nil, rerr
 			}

internal/providers/providerio/providerio.go

@@ -16,7 +16,6 @@ import (
 	"strings"
 	"time"
 
-	"github.com/Gitlawb/zero/internal/trace"
 	"github.com/Gitlawb/zero/internal/zeroruntime"
 )
 
@@ -372,15 +371,9 @@ func ScanSSEDataWithContext(
 				// The provider asked to stop (e.g. it already emitted an error
 				// for this payload). Abort the read and end like ScanSSEData:
 				// return nil so callers fall through to their post-scan checks.
-				// Do not stamp FirstToken — this payload was not accepted model
-				// output, so counting it as first-token time would be misleading.
 				cancel()
 				return nil
 			}
-			// First accepted non-keepalive payload = first real model output. Stamp
-			// once; later real payloads are no-ops. nil recorder (untraced run) is a
-			// no-op.
-			trace.FromContext(ctx).StampFirstToken()
 		}
 	}
 }

internal/providers/providerio/retry.go

@@ -7,8 +7,6 @@ import (
 	"strconv"
 	"strings"
 	"time"
-
-	"github.com/Gitlawb/zero/internal/trace"
 )
 
 // Transient-failure retry, shared by every provider.
@@ -69,9 +67,7 @@ func SendWithRetry(
 			setHeader(request)
 		}
 
-		connectSpan := trace.FromContext(ctx).Span(trace.SpanProviderConnect)
 		response, err := client.Do(request)
-		connectSpan.End()
 		if err != nil {
 			// A transport failure on a POST does NOT mean the server didn't receive
 			// it — the request may have arrived and be generating a (billable,
@@ -86,9 +82,6 @@ func SendWithRetry(
 		}
 
 		if ShouldRetryStatus(response.StatusCode) && attempt < maxAttempts {
-			if r := trace.FromContext(ctx); r != nil {
-				r.Counter(trace.CounterRetryCount, 1)
-			}
 			wait := RetryAfter(response)
 			_ = response.Body.Close()
 			if Backoff(ctx, attempt, wait) {

internal/sessions/store.go

@@ -35,16 +35,21 @@ const (
 	EventProviderUsage      EventType = "provider_usage"
 	EventUsage              EventType = EventProviderUsage
 	EventError              EventType = "error"
-	EventSessionCheckpoint  EventType = "session_checkpoint"
-	EventSessionRewind      EventType = "session_rewind"
-	EventCompaction         EventType = "session_compaction"
-	EventSessionFork        EventType = "session_fork"
-	EventSessionChild       EventType = "session_child"
-	EventSpecialistStart    EventType = "specialist_start"
-	EventSpecialistStop     EventType = "specialist_stop"
-	EventSpecDraft          EventType = "spec_draft"
-	EventSpecApproved       EventType = "spec_approved"
-	EventSpecRejected       EventType = "spec_rejected"
+	// EventRunIncomplete records a run that stopped with work clearly unfinished
+	// (plan stall, mid-step stop, max-turns cutoff) without a provider/tool error.
+	// Distinct from EventError so TUI resume replays it as a system notice, not a
+	// hard failure.
+	EventRunIncomplete     EventType = "run_incomplete"
+	EventSessionCheckpoint EventType = "session_checkpoint"
+	EventSessionRewind     EventType = "session_rewind"
+	EventCompaction        EventType = "session_compaction"
+	EventSessionFork       EventType = "session_fork"
+	EventSessionChild      EventType = "session_child"
+	EventSpecialistStart   EventType = "specialist_start"
+	EventSpecialistStop    EventType = "specialist_stop"
+	EventSpecDraft         EventType = "spec_draft"
+	EventSpecApproved      EventType = "spec_approved"
+	EventSpecRejected      EventType = "spec_rejected"
 )
 
 type SessionKind string

internal/skills/install.go

@@ -221,52 +221,6 @@ func Info(dir string, name string) (SkillInfo, bool) {
 	return info, true
 }
 
-// InfoFromRoots resolves the named skill across discovery roots (earlier roots
-// win). Lock source/hash are attached only when the winning skill lives under
-// primaryDir and that dir's lockfile has an entry — agents-only skills return
-// frontmatter + path with empty Source/Hash. primaryDir is typically the Zero
-// write root (DefaultDir / skillsDir).
-func InfoFromRoots(primaryDir string, roots []string, name string) (SkillInfo, bool) {
-	loaded, _, err := LoadFromRoots(roots)
-	if err != nil {
-		return SkillInfo{}, false
-	}
-	target := strings.TrimSpace(name)
-	var skill Skill
-	found := false
-	for _, candidate := range loaded {
-		if candidate.Name == target {
-			skill = candidate
-			found = true
-			break
-		}
-	}
-	if !found {
-		return SkillInfo{}, false
-	}
-	info := SkillInfo{Skill: skill}
-	primaryDir = strings.TrimSpace(primaryDir)
-	if primaryDir == "" {
-		return info, true
-	}
-	// Only attach lock metadata when the winner is from the primary write root.
-	// Compare path prefixes after cleaning so agents-only skills never pick up a
-	// Zero lock entry by name coincidence.
-	skillPath := filepath.Clean(skill.Path)
-	primaryRoot := filepath.Clean(primaryDir)
-	rel, err := filepath.Rel(primaryRoot, skillPath)
-	if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) || filepath.IsAbs(rel) {
-		return info, true
-	}
-	if lock, err := ReadLock(primaryDir); err == nil {
-		if entry, found := lock[skill.Name]; found {
-			info.Source = entry.Source
-			info.Hash = entry.Hash
-		}
-	}
-	return info, true
-}
-
 // ReadLock loads the lockfile from dir. A missing lockfile yields an empty map
 // with no error so callers can treat "no lockfile" as "nothing installed".
 func ReadLock(dir string) (map[string]LockEntry, error) {

internal/skills/skills.go

@@ -28,18 +28,10 @@ type Skill struct {
 
 const skillFileName = "SKILL.md"
 
-// errNotDirectory is returned when a skills root path exists but is not a
-// directory. On Windows, os.ReadDir reports that case as ErrNotExist
-// (ENOTDIR aliases ERROR_PATH_NOT_FOUND), so load reclassifies it explicitly.
-var errNotDirectory = errors.New("not a directory")
-
 // DefaultDir resolves the skills directory, mirroring sessions.DefaultRoot. An
 // explicit ZERO_SKILLS_DIR override wins; otherwise it is
 // $XDG_DATA_HOME/zero/skills or ~/.local/share/zero/skills. The directory is
 // NOT created — a missing directory simply yields no skills.
-//
-// DefaultDir is the primary write root for install/remove/lock. Runtime discovery
-// also considers AgentsDir and plugin skill roots via DiscoveryRoots / LoadFromRoots.
 func DefaultDir(env map[string]string) string {
 	if override := strings.TrimSpace(envValue(env, "ZERO_SKILLS_DIR")); override != "" {
 		return override
@@ -64,66 +56,6 @@ func DefaultDir(env map[string]string) string {
 	return filepath.Join(base, "zero", "skills")
 }
 
-// AgentsDir returns ~/.agents/skills when that path exists and is a directory.
-// It is a shared, read-only multi-agent skills root (Zero, Hermes, Claude Code,
-// etc.) and is never the target of install/remove/lock. Missing, non-directory,
-// or unresolvable home yields "" with no error and no directory creation.
-//
-// Home resolution matches other packages: HOME, then USERPROFILE, then
-// os.UserHomeDir(). ZERO_SKILLS_DIR is intentionally ignored — agents is a
-// pure convention path, not a Zero-specific override.
-func AgentsDir(env map[string]string) string {
-	home := strings.TrimSpace(firstNonEmpty(
-		envValue(env, "HOME"),
-		envValue(env, "USERPROFILE"),
-	))
-	if home == "" {
-		userHome, err := os.UserHomeDir()
-		if err != nil || strings.TrimSpace(userHome) == "" {
-			return ""
-		}
-		home = userHome
-	}
-	dir := filepath.Join(home, ".agents", "skills")
-	info, err := os.Stat(dir)
-	if err != nil || !info.IsDir() {
-		return ""
-	}
-	return dir
-}
-
-// DiscoveryRoots returns ordered skill roots for runtime discovery: primary
-// DefaultDir, optional AgentsDir when present, then pluginRoots. Empty strings
-// are omitted. Earlier entries win on name clashes.
-func DiscoveryRoots(env map[string]string, pluginRoots []string) []string {
-	return collectRoots(DefaultDir(env), AgentsDir(env), pluginRoots)
-}
-
-// collectRoots assembles ordered non-empty skill roots. primary is typically
-// DefaultDir (or an injected test dir); agents is typically AgentsDir's result.
-func collectRoots(primary string, agents string, pluginRoots []string) []string {
-	roots := make([]string, 0, 2+len(pluginRoots))
-	if primary = strings.TrimSpace(primary); primary != "" {
-		roots = append(roots, primary)
-	}
-	if agents = strings.TrimSpace(agents); agents != "" {
-		roots = append(roots, agents)
-	}
-	for _, root := range pluginRoots {
-		if root = strings.TrimSpace(root); root != "" {
-			roots = append(roots, root)
-		}
-	}
-	return roots
-}
-
-// GlobalRoots returns discovery roots for management CLI list/info: an explicit
-// primary write/root dir (usually skillsDir / DefaultDir) plus AgentsDir when
-// present. Plugin roots are excluded from management UX.
-func GlobalRoots(primary string) []string {
-	return collectRoots(primary, AgentsDir(nil), nil)
-}
-
 // DuplicateName records two skills that resolved to the same frontmatter name.
 // Winner is the SKILL.md path of the skill that was kept (the one in the
 // lexicographically-first directory); Loser is the path that was dropped.
@@ -145,81 +77,14 @@ type DuplicateName struct {
 // winner regardless of sort stability. Use Duplicates to surface a warning about
 // any such collisions.
 //
-// NOTE: Load scans one root. Runtime discovery uses LoadFromRoots /
-// DiscoveryRoots (primary DefaultDir, optional ~/.agents/skills, then plugin
-// skill roots). Prefer those multi-root helpers for agent/CLI discovery; keep
-// Load for single-dir install/write call sites.
+// NOTE: Load scans one root. Agent startup merges plugin-declared skill roots
+// separately during plugin activation, so the runtime skill surface can include
+// both the default directory and skills bundled by active plugins.
 func Load(dir string) ([]Skill, error) {
 	skills, _, err := load(dir)
 	return skills, err
 }
 
-// LoadFromRoots loads and merges skills from the provided directories (earlier
-// entries win on name clashes). Empty roots are skipped. Missing directories are
-// treated as empty (same as Load). Intra-root and cross-root collisions are
-// reported as DuplicateName.
-//
-// The first non-empty root is the required primary: non-missing load failures
-// (permission, I/O, not a directory, etc.) are returned so callers do not
-// confuse a broken primary skills dir with "no skills". Later optional roots
-// (e.g. ~/.agents/skills, plugin roots) fail open so one bad optional directory
-// does not hide the rest.
-func LoadFromRoots(dirs []string) ([]Skill, []DuplicateName, error) {
-	merged := make([]Skill, 0)
-	duplicates := []DuplicateName{}
-	byName := map[string]int{}
-	primary := true
-
-	for _, dir := range dirs {
-		if strings.TrimSpace(dir) == "" {
-			continue
-		}
-		loaded, rootDups, err := load(dir)
-		if err != nil {
-			if primary {
-				// Required primary root: surface real I/O failures instead of
-				// collapsing to an empty skill list.
-				return nil, nil, err
-			}
-			// Optional roots fail open: one bad directory must not hide the rest.
-			continue
-		}
-		primary = false
-		duplicates = append(duplicates, rootDups...)
-		for _, skill := range loaded {
-			if winnerIdx, clash := byName[skill.Name]; clash {
-				duplicates = append(duplicates, DuplicateName{
-					Name:   skill.Name,
-					Winner: merged[winnerIdx].Path,
-					Loser:  skill.Path,
-				})
-				continue
-			}
-			byName[skill.Name] = len(merged)
-			merged = append(merged, skill)
-		}
-	}
-
-	sort.Slice(merged, func(left int, right int) bool {
-		return merged[left].Name < merged[right].Name
-	})
-	return merged, duplicates, nil
-}
-
-// ListFromRoots is like LoadFromRoots but strips Content (like List).
-func ListFromRoots(dirs []string) ([]Skill, []DuplicateName, error) {
-	loaded, dups, err := LoadFromRoots(dirs)
-	if err != nil {
-		return nil, dups, err
-	}
-	listed := make([]Skill, 0, len(loaded))
-	for _, skill := range loaded {
-		skill.Content = ""
-		listed = append(listed, skill)
-	}
-	return listed, dups, nil
-}
-
 // Duplicates returns the duplicate-name collisions Load resolved by the
 // first-directory-wins rule, so a caller can warn the user that a shadowed skill
 // was dropped. A missing directory yields no duplicates and no error.
@@ -266,16 +131,6 @@ func load(dir string) ([]Skill, []DuplicateName, error) {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
 		if errors.Is(err, os.ErrNotExist) {
-			// Windows maps ENOTDIR to ERROR_PATH_NOT_FOUND, which is also
-			// ErrNotExist. A primary skills path that exists but is not a
-			// directory must still surface as a load error, not "no skills".
-			if info, statErr := os.Stat(dir); statErr == nil && !info.IsDir() {
-				return nil, nil, &os.PathError{
-					Op:   "readdir",
-					Path: dir,
-					Err:  errNotDirectory,
-				}
-			}
 			return []Skill{}, nil, nil
 		}
 		return nil, nil, err
@@ -426,7 +281,7 @@ func splitFrontmatter(normalized string) (string, string, bool) {
 // Matching is case-insensitive on the key; the first occurrence wins.
 func frontmatterValue(frontmatter string, key string) string {
 	prefix := strings.ToLower(key) + ":"
-	for line := range strings.SplitSeq(frontmatter, "\n") {
+	for _, line := range strings.Split(frontmatter, "\n") {
 		trimmed := strings.TrimSpace(line)
 		if strings.HasPrefix(strings.ToLower(trimmed), prefix) {
 			value := strings.TrimSpace(trimmed[len(prefix):])
@@ -442,12 +297,3 @@ func envValue(env map[string]string, key string) string {
 	}
 	return os.Getenv(key)
 }
-
-func firstNonEmpty(values ...string) string {
-	for _, value := range values {
-		if strings.TrimSpace(value) != "" {
-			return value
-		}
-	}
-	return ""
-}

internal/skills/skills_test.go

@@ -1,7 +1,6 @@
 package skills
 
 import (
-	"errors"
 	"os"
 	"path/filepath"
 	"strings"
@@ -148,25 +147,6 @@ func TestLoadMissingDirYieldsEmpty(t *testing.T) {
 	}
 }
 
-func TestLoadNotDirectoryErrors(t *testing.T) {
-	// Portable across Unix and Windows: a regular file is not a skills root.
-	// Windows reports ReadDir(file) as ErrNotExist; load must reclassify it.
-	notDir := filepath.Join(t.TempDir(), "not-a-dir")
-	if err := os.WriteFile(notDir, []byte("x"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	loaded, err := Load(notDir)
-	if err == nil {
-		t.Fatalf("expected error for non-directory skills root, got loaded=%#v", loaded)
-	}
-	if errors.Is(err, os.ErrNotExist) && !errors.Is(err, errNotDirectory) {
-		t.Fatalf("non-directory skills root must not look missing: %v", err)
-	}
-	if loaded != nil {
-		t.Fatalf("expected nil skills on non-directory root, got %#v", loaded)
-	}
-}
-
 func TestLoadSortsByName(t *testing.T) {
 	dir := t.TempDir()
 	writeSkill(t, dir, "zeta", "body")
@@ -312,273 +292,3 @@ func TestDefaultDirFallsBackToHome(t *testing.T) {
 		t.Fatalf("DefaultDir = %q, want %q", got, want)
 	}
 }
-
-func TestAgentsDirReturnsExistingDirectory(t *testing.T) {
-	home := t.TempDir()
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	got := AgentsDir(map[string]string{"HOME": home})
-	if got != agents {
-		t.Fatalf("AgentsDir = %q, want %q", got, agents)
-	}
-}
-
-func TestAgentsDirMissingIsEmpty(t *testing.T) {
-	home := t.TempDir()
-	got := AgentsDir(map[string]string{"HOME": home})
-	if got != "" {
-		t.Fatalf("AgentsDir for missing path = %q, want empty", got)
-	}
-}
-
-func TestAgentsDirFileNotDirIsEmpty(t *testing.T) {
-	home := t.TempDir()
-	agentsParent := filepath.Join(home, ".agents")
-	if err := os.MkdirAll(agentsParent, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	// skills is a file, not a directory
-	if err := os.WriteFile(filepath.Join(agentsParent, "skills"), []byte("not a dir"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	got := AgentsDir(map[string]string{"HOME": home})
-	if got != "" {
-		t.Fatalf("AgentsDir for file = %q, want empty", got)
-	}
-}
-
-func TestAgentsDirHonorsUserProfile(t *testing.T) {
-	home := t.TempDir()
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	got := AgentsDir(map[string]string{"USERPROFILE": home})
-	if got != agents {
-		t.Fatalf("AgentsDir via USERPROFILE = %q, want %q", got, agents)
-	}
-}
-
-func TestAgentsDirIgnoresZeroSkillsDir(t *testing.T) {
-	home := t.TempDir()
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	// ZERO_SKILLS_DIR must not redirect or suppress AgentsDir.
-	got := AgentsDir(map[string]string{
-		"HOME":            home,
-		"ZERO_SKILLS_DIR": filepath.Join(home, "zero-only"),
-	})
-	if got != agents {
-		t.Fatalf("AgentsDir with ZERO_SKILLS_DIR set = %q, want %q", got, agents)
-	}
-}
-
-func TestAgentsDirUnresolvableHomeIsEmpty(t *testing.T) {
-	// Empty env map values + no real UserHomeDir fallback is hard to force, but
-	// empty HOME/USERPROFILE with an empty home should still not panic.
-	// When UserHomeDir works this may return a host path; only assert no panic
-	// and that a deliberately empty-looking override path is not invented from ZERO.
-	_ = AgentsDir(map[string]string{"HOME": "", "USERPROFILE": ""})
-}
-
-func TestDiscoveryRootsOrderAndOmission(t *testing.T) {
-	home := t.TempDir()
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	primary := filepath.Join(home, "zero-skills")
-	env := map[string]string{
-		"HOME":            home,
-		"ZERO_SKILLS_DIR": primary,
-	}
-	roots := DiscoveryRoots(env, []string{"", " /plugin/a ", "plugin/b"})
-	want := []string{primary, agents, "/plugin/a", "plugin/b"}
-	if len(roots) != len(want) {
-		t.Fatalf("DiscoveryRoots = %#v, want %#v", roots, want)
-	}
-	for i := range want {
-		if roots[i] != want[i] {
-			t.Fatalf("DiscoveryRoots[%d] = %q, want %q (full %#v)", i, roots[i], want[i], roots)
-		}
-	}
-}
-
-func TestDiscoveryRootsOmitsMissingAgents(t *testing.T) {
-	home := t.TempDir()
-	primary := filepath.Join(home, "zero-skills")
-	env := map[string]string{
-		"HOME":            home,
-		"ZERO_SKILLS_DIR": primary,
-	}
-	roots := DiscoveryRoots(env, nil)
-	if len(roots) != 1 || roots[0] != primary {
-		t.Fatalf("DiscoveryRoots without agents = %#v, want only primary", roots)
-	}
-}
-
-func TestLoadFromRootsPrimaryWinsOverAgents(t *testing.T) {
-	primary := t.TempDir()
-	agents := t.TempDir()
-	writeSkill(t, primary, "shared", "---\nname: shared\n---\nprimary body\n")
-	writeSkill(t, agents, "shared", "---\nname: shared\n---\nagents body\n")
-	writeSkill(t, agents, "agents-only", "---\nname: agents-only\n---\nagents only\n")
-
-	loaded, dups, err := LoadFromRoots([]string{primary, agents})
-	if err != nil {
-		t.Fatalf("LoadFromRoots: %v", err)
-	}
-	byName := map[string]Skill{}
-	for _, skill := range loaded {
-		byName[skill.Name] = skill
-	}
-	if byName["shared"].Content != "primary body" {
-		t.Fatalf("primary should win shared, got %q", byName["shared"].Content)
-	}
-	if byName["agents-only"].Content != "agents only" {
-		t.Fatalf("agents-only missing: %#v", loaded)
-	}
-	if len(dups) != 1 || dups[0].Name != "shared" {
-		t.Fatalf("expected one shared duplicate, got %#v", dups)
-	}
-}
-
-func TestLoadFromRootsSkipsEmptyAndMissing(t *testing.T) {
-	primary := t.TempDir()
-	writeSkill(t, primary, "solo", "---\nname: solo\n---\nbody\n")
-	loaded, dups, err := LoadFromRoots([]string{"", filepath.Join(t.TempDir(), "missing"), primary})
-	if err != nil {
-		t.Fatalf("LoadFromRoots: %v", err)
-	}
-	if len(loaded) != 1 || loaded[0].Name != "solo" {
-		t.Fatalf("expected only solo, got %#v", loaded)
-	}
-	if len(dups) != 0 {
-		t.Fatalf("unexpected dups: %#v", dups)
-	}
-}
-
-func TestLoadFromRootsBubblesPrimaryError(t *testing.T) {
-	// A regular file is not a directory. On Unix ReadDir fails with ENOTDIR; on
-	// Windows that case is aliased to ErrNotExist, so load must reclassify an
-	// existing non-directory and bubble it from the first non-empty root.
-	notDir := filepath.Join(t.TempDir(), "not-a-dir")
-	if err := os.WriteFile(notDir, []byte("x"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	optional := t.TempDir()
-	writeSkill(t, optional, "fallback", "---\nname: fallback\n---\nbody\n")
-
-	loaded, dups, err := LoadFromRoots([]string{notDir, optional})
-	if err == nil {
-		t.Fatalf("expected primary root error, got loaded=%#v dups=%#v", loaded, dups)
-	}
-	// Unix returns ENOTDIR; Windows reclassifies via errNotDirectory because
-	// ENOTDIR aliases ErrNotExist there. Either way it must not look missing.
-	if errors.Is(err, os.ErrNotExist) && !errors.Is(err, errNotDirectory) {
-		t.Fatalf("primary non-directory must not look missing: %v", err)
-	}
-	if loaded != nil {
-		t.Fatalf("expected nil skills on primary error, got %#v", loaded)
-	}
-	if dups != nil {
-		t.Fatalf("expected nil dups on primary error, got %#v", dups)
-	}
-}
-
-func TestLoadFromRootsOptionalRootFailOpen(t *testing.T) {
-	primary := t.TempDir()
-	writeSkill(t, primary, "keep", "---\nname: keep\n---\nbody\n")
-	notDir := filepath.Join(t.TempDir(), "not-a-dir")
-	if err := os.WriteFile(notDir, []byte("x"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	agents := t.TempDir()
-	writeSkill(t, agents, "agents-only", "---\nname: agents-only\n---\nbody\n")
-
-	loaded, dups, err := LoadFromRoots([]string{primary, notDir, agents})
-	if err != nil {
-		t.Fatalf("optional root failure must not fail merge: %v", err)
-	}
-	byName := map[string]Skill{}
-	for _, skill := range loaded {
-		byName[skill.Name] = skill
-	}
-	if byName["keep"].Name != "keep" {
-		t.Fatalf("primary skill missing: %#v", loaded)
-	}
-	if byName["agents-only"].Name != "agents-only" {
-		t.Fatalf("later optional skill should still load: %#v", loaded)
-	}
-	if len(dups) != 0 {
-		t.Fatalf("unexpected dups: %#v", dups)
-	}
-}
-
-func TestListFromRootsStripsContent(t *testing.T) {
-	dir := t.TempDir()
-	writeSkill(t, dir, "demo", "---\nname: demo\n---\nbody content\n")
-	listed, _, err := ListFromRoots([]string{dir})
-	if err != nil {
-		t.Fatalf("ListFromRoots: %v", err)
-	}
-	if len(listed) != 1 || listed[0].Name != "demo" {
-		t.Fatalf("unexpected listed: %#v", listed)
-	}
-	if listed[0].Content != "" {
-		t.Fatalf("ListFromRoots must strip Content, got %q", listed[0].Content)
-	}
-}
-
-func TestGlobalRootsIncludesAgents(t *testing.T) {
-	home := t.TempDir()
-	agents := filepath.Join(home, ".agents", "skills")
-	if err := os.MkdirAll(agents, 0o755); err != nil {
-		t.Fatal(err)
-	}
-	// Point HOME so AgentsDir finds the temp agents root.
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	primary := filepath.Join(home, "primary")
-	roots := GlobalRoots(primary)
-	if len(roots) != 2 || roots[0] != primary || roots[1] != agents {
-		t.Fatalf("GlobalRoots = %#v, want [primary, agents]", roots)
-	}
-}
-
-func TestInfoFromRootsAgentsOnlyHasNoLock(t *testing.T) {
-	primary := t.TempDir()
-	agents := t.TempDir()
-	writeSkill(t, agents, "shared-agents", "---\nname: agents-skill\ndescription: from agents\n---\nbody\n")
-	info, ok := InfoFromRoots(primary, []string{primary, agents}, "agents-skill")
-	if !ok {
-		t.Fatal("expected agents skill to resolve")
-	}
-	if info.Skill.Name != "agents-skill" || info.Skill.Description != "from agents" {
-		t.Fatalf("unexpected skill: %#v", info.Skill)
-	}
-	if info.Source != "" || info.Hash != "" {
-		t.Fatalf("agents-only skill must not carry lock metadata, got source=%q hash=%q", info.Source, info.Hash)
-	}
-}
-
-func TestInfoFromRootsPrimaryLockMetadata(t *testing.T) {
-	primary := t.TempDir()
-	writeSkill(t, primary, "demo", "---\nname: demo\n---\nbody\n")
-	// Write a lockfile entry the way install would.
-	lockPath := filepath.Join(primary, LockFileName)
-	if err := os.WriteFile(lockPath, []byte(`{"demo":{"source":"file:///src","hash":"sha256:abc"}}`), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	info, ok := InfoFromRoots(primary, []string{primary}, "demo")
-	if !ok {
-		t.Fatal("expected primary skill")
-	}
-	if info.Source != "file:///src" || info.Hash != "sha256:abc" {
-		t.Fatalf("lock metadata missing: %#v", info)
-	}
-}

internal/swarm/scheduler_test.go

@@ -85,14 +85,6 @@ func TestSchedulerSkipsWhilePreviousRuns(t *testing.T) {
 	waitFor(t, "first done", func() bool { return sw.Coordinator().Summarize().Done == 1 })
 	ticks <- time.Time{}
 	waitFor(t, "second spawn", func() bool { return len(l.recorded()) == 2 })
-	// fireIfIdle's spawn (what "second spawn" observes via the launcher) and
-	// run's subsequent job.incRuns() are sequential but distinct steps in the
-	// scheduler's goroutine; wait for Runs itself rather than assuming the
-	// launcher recording it means the job's counter is updated too.
-	waitFor(t, "second run recorded", func() bool {
-		j, ok := findJob(sched.List(), id)
-		return ok && j.Runs == 2
-	})
 
 	j, ok := findJob(sched.List(), id)
 	if !ok {

internal/tools/apply_patch.go

@@ -34,8 +34,7 @@ func NewScopedApplyPatchTool(workspaceRoot string, scope PathScope) Tool {
 				Required:             []string{"patch"},
 				AdditionalProperties: false,
 			},
-			safety:       promptSafety(SideEffectWrite, "Applies patch hunks that can create, edit, or delete files."),
-			capabilities: ToolCapabilities{Effect: EffectWorkspaceWrite, ThreadSafe: false, ResourceKeys: applyPatchResourceKeys},
+			safety: promptSafety(SideEffectWrite, "Applies patch hunks that can create, edit, or delete files."),
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/ask_user.go

@@ -88,8 +88,6 @@ func NewAskUserTool() *askUserTool {
 				AdditionalProperties: false,
 			},
 			safety: readOnlySafety("Asks the user clarifying questions; gathers input only."),
-			// EffectInteractive: blocks on user input; ordering matters.
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false},
 		},
 	}
 }

internal/tools/bash.go

@@ -52,10 +52,6 @@ func NewScopedBashTool(workspaceRoot string, scope PathScope) Tool {
 				AdditionalProperties: false,
 			},
 			safety: promptSafety(SideEffectShell, "Shell commands can read, write, or execute programs."),
-			// One-shot shell can rewrite the workspace (and more). Serialized
-			// as WorkspaceWrite so external-mutator audits include bash; not
-			// session-bound Interactive (that is reserved for PTY/terminal tools).
-			capabilities: ToolCapabilities{Effect: EffectWorkspaceWrite, ThreadSafe: false, ResourceKeys: directoryResourceKeys},
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/builtin_catalog.go

@@ -1,94 +0,0 @@
-package tools
-
-import "github.com/Gitlawb/zero/internal/localcontrol"
-
-// BuiltinCatalog returns every built-in tool that Zero can expose to the model
-// from this package (core + control + optional local-control helpers).
-//
-// It is the source of truth for metadata coverage tests: every tool returned
-// here must declare an explicit Effect other than EffectUnknown.
-//
-// Not included (fail-closed EffectUnknown by design):
-//   - MCP tools (internal/mcp registryTool)
-//   - Plugin tools (internal/plugins pluginTool)
-//   - Specialist tools registered outside this package (Task, Generate, …)
-//
-// workspaceRoot is used only to construct path-scoped tools; it need not exist.
-func BuiltinCatalog(workspaceRoot string) []Tool {
-	if workspaceRoot == "" {
-		workspaceRoot = "."
-	}
-	var all []Tool
-	all = append(all, CoreReadOnlyTools(workspaceRoot)...)
-	all = append(all, CoreWriteTools(workspaceRoot)...)
-	all = append(all, CoreShellTools(workspaceRoot)...)
-	// Network tools: always include web_fetch; always include web_search for
-	// classification even when no backend is configured at runtime (registration
-	// remains conditional in CoreNetworkTools).
-	all = append(all, NewWebFetchTool(), NewWebSearchTool())
-	all = append(all, NewEscalateModelTool())
-
-	// tool_search needs a live registry of the tools above.
-	reg := NewRegistry()
-	for _, tool := range all {
-		reg.Register(tool)
-	}
-	search := NewToolSearchTool(reg)
-	reg.Register(search)
-	all = append(all, search)
-
-	// Local-control tools are feature-gated at registration time but always
-	// exist as built-in implementations with fixed classifications.
-	all = append(all, NewLocalBrowserTools(localcontrol.BrowserOptions{})...)
-	all = append(all, NewLocalDesktopTools(localcontrol.DesktopOptions{})...)
-	all = append(all, NewLocalTerminalTools(localcontrol.TerminalOptions{})...)
-	all = append(all, NewLocalControlArtifactTools(LocalControlArtifactOptions{})...)
-	return all
-}
-
-// ValidateBuiltinCatalog returns diagnostics for every catalog tool that is
-// missing or inconsistently classified. Empty means full coverage.
-//
-// Duplicate builtin names are always rejected: ResourceKeys is a function and
-// cannot be compared, so partial metadata comparison would miss conflict-scope
-// drift between two same-name tools.
-//
-// Validation uses constructor-declared capabilities (before ThreadSafe
-// normalization) so a mutator wired with ThreadSafe=true is still reported.
-// Runtime callers continue to use CapabilitiesOf, which normalizes.
-func ValidateBuiltinCatalog(workspaceRoot string) []string {
-	var problems []string
-	seen := map[string]struct{}{}
-	for _, tool := range BuiltinCatalog(workspaceRoot) {
-		name := tool.Name()
-		if _, ok := seen[name]; ok {
-			problems = append(problems, name+": duplicate builtin name in catalog")
-			continue
-		}
-		seen[name] = struct{}{}
-
-		// Runtime view: must not remain Unknown after normalization.
-		runtimeCaps := CapabilitiesOf(tool)
-		if runtimeCaps.Effect == EffectUnknown {
-			problems = append(problems, name+": built-in tool remains EffectUnknown (metadata required)")
-		}
-
-		// Declaration view: catch invalid ThreadSafe on mutators that
-		// CapabilitiesOf would silently clear.
-		declared := declaredCapabilitiesOf(tool)
-		problems = append(problems, ValidateCapabilities(name, declared)...)
-	}
-	return problems
-}
-
-// declaredCapabilitiesOf returns constructor metadata when available; otherwise
-// falls back to CapabilitiesOf (already normalized).
-func declaredCapabilitiesOf(tool Tool) ToolCapabilities {
-	if tool == nil {
-		return UnknownCapabilities()
-	}
-	if d, ok := tool.(declaredCapabilityProvider); ok {
-		return d.declaredCapabilities()
-	}
-	return CapabilitiesOf(tool)
-}

internal/tools/capabilities.go

@@ -1,172 +0,0 @@
-package tools
-
-import "strings"
-
-// EffectClass classifies a tool's side effects for concurrency safety.
-// Concurrent execution (a later PR) must only consider tools that are both
-// ThreadSafe and EffectReadOnly; this package only declares metadata.
-// ThreadSafe is force-cleared for every non-ReadOnly effect at read time
-// (see normalizeCapabilities).
-type EffectClass int
-
-const (
-	// EffectUnknown is the fail-closed default for unclassified tools.
-	// Never concurrency-eligible; always serialized.
-	EffectUnknown EffectClass = iota
-	// EffectReadOnly reads local or remote state without mutating it.
-	// The only effect class that may set ThreadSafe=true after audit.
-	EffectReadOnly
-	// EffectProcessRead observes a retained process, PTY, or terminal.
-	EffectProcessRead
-	// EffectWorkspaceWrite mutates the local repository or workspace.
-	// Includes one-shot shell commands that can rewrite the workspace.
-	EffectWorkspaceWrite
-	// EffectExternalWrite mutates state outside the local workspace.
-	EffectExternalWrite
-	// EffectInteractive uses browser, PTY/terminal, prompts, or other
-	// session-bound interaction where ordering matters.
-	EffectInteractive
-)
-
-// String returns a stable diagnostic name for the effect class.
-func (e EffectClass) String() string {
-	switch e {
-	case EffectUnknown:
-		return "unknown"
-	case EffectReadOnly:
-		return "read_only"
-	case EffectProcessRead:
-		return "process_read"
-	case EffectWorkspaceWrite:
-		return "workspace_write"
-	case EffectExternalWrite:
-		return "external_write"
-	case EffectInteractive:
-		return "interactive"
-	default:
-		return "invalid"
-	}
-}
-
-// Valid reports whether e is one of the defined effect classes.
-func (e EffectClass) Valid() bool {
-	return e >= EffectUnknown && e <= EffectInteractive
-}
-
-// ToolCapabilities is the concurrency-safety contract for a tool.
-// ResourceKeys, when set, returns deterministic conflict keys for a call's
-// arguments. Returning no keys does NOT make a tool parallel-safe.
-type ToolCapabilities struct {
-	Effect       EffectClass
-	ThreadSafe   bool
-	ResourceKeys func(args map[string]any) []string
-}
-
-// CapabilityProvider is the optional interface tools implement to declare
-// effect metadata. Tools that do not implement it are treated as EffectUnknown
-// and not thread-safe (fail-closed).
-type CapabilityProvider interface {
-	Capabilities() ToolCapabilities
-}
-
-// UnknownCapabilities is the fail-closed default for tools without metadata.
-func UnknownCapabilities() ToolCapabilities {
-	return ToolCapabilities{
-		Effect:     EffectUnknown,
-		ThreadSafe: false,
-	}
-}
-
-// normalizeCapabilities is the single read path for capability metadata.
-// ThreadSafe is cleared unless Effect is a valid EffectReadOnly — mutators,
-// interactive tools, process readers, unknown, and invalid effects must never
-// appear concurrency-eligible to a future batch planner that only checks
-// ThreadSafe && Effect.
-func normalizeCapabilities(caps ToolCapabilities) ToolCapabilities {
-	if !caps.Effect.Valid() || caps.Effect != EffectReadOnly {
-		caps.ThreadSafe = false
-	}
-	return caps
-}
-
-// CapabilitiesOf returns a tool's declared capabilities, or the fail-closed
-// unknown default when the tool does not implement CapabilityProvider.
-func CapabilitiesOf(tool Tool) ToolCapabilities {
-	if tool == nil {
-		return UnknownCapabilities()
-	}
-	if provider, ok := tool.(CapabilityProvider); ok {
-		return normalizeCapabilities(provider.Capabilities())
-	}
-	return UnknownCapabilities()
-}
-
-// (baseTool).Capabilities exposes the capabilities field set at construction,
-// normalized so ThreadSafe cannot surface on non-ReadOnly effects.
-// Tools that embed baseTool inherit this method.
-func (tool baseTool) Capabilities() ToolCapabilities {
-	return normalizeCapabilities(tool.capabilities)
-}
-
-// declaredCapabilities returns the constructor-set metadata without runtime
-// normalization. Catalog validation uses this so a mis-declared
-// ThreadSafe=true on a mutator is still reported even though CapabilitiesOf
-// clears it for callers.
-func (tool baseTool) declaredCapabilities() ToolCapabilities {
-	return tool.capabilities
-}
-
-// declaredCapabilityProvider is implemented by tools that can report their
-// raw constructor metadata (baseTool embedders).
-type declaredCapabilityProvider interface {
-	declaredCapabilities() ToolCapabilities
-}
-
-// Registry.Capabilities looks up a registered tool's capabilities by name.
-// Unknown tools return the fail-closed default (not an error) so callers can
-// treat missing tools as non-concurrent without panicking.
-func (registry *Registry) Capabilities(name string) ToolCapabilities {
-	if registry == nil {
-		return UnknownCapabilities()
-	}
-	tool, ok := registry.Get(name)
-	if !ok {
-		return UnknownCapabilities()
-	}
-	return CapabilitiesOf(tool)
-}
-
-// ValidateCapabilities checks a single capability record for consistency.
-// It returns a non-empty slice of human-readable problems when invalid.
-// Note: CapabilitiesOf already normalizes ThreadSafe away for non-ReadOnly
-// effects; validation still rejects the raw combination so catalog tests
-// catch mis-declared constructors.
-func ValidateCapabilities(name string, caps ToolCapabilities) []string {
-	var problems []string
-	name = strings.TrimSpace(name)
-	if name == "" {
-		name = "<unnamed>"
-	}
-	if !caps.Effect.Valid() {
-		problems = append(problems, name+": invalid effect value")
-	}
-	if caps.Effect == EffectUnknown && caps.ThreadSafe {
-		problems = append(problems, name+": ThreadSafe=true is forbidden with EffectUnknown")
-	}
-	// Mutating / interactive / process-read effects are never concurrency-eligible.
-	switch caps.Effect {
-	case EffectWorkspaceWrite, EffectExternalWrite, EffectInteractive, EffectProcessRead:
-		if caps.ThreadSafe {
-			problems = append(problems, name+": ThreadSafe=true is not permitted for effect "+caps.Effect.String())
-		}
-	}
-	return problems
-}
-
-// ValidateToolCapabilities validates CapabilitiesOf(tool) for a registered tool.
-func ValidateToolCapabilities(tool Tool) []string {
-	if tool == nil {
-		return []string{"<nil tool>"}
-	}
-	return ValidateCapabilities(tool.Name(), CapabilitiesOf(tool))
-}

internal/tools/capabilities_test.go

@@ -1,449 +0,0 @@
-package tools
-
-import (
-	"context"
-	"path/filepath"
-	"runtime"
-	"strings"
-	"sync"
-	"testing"
-)
-
-// plainTool implements Tool without CapabilityProvider — fail-closed default.
-type plainTool struct {
-	name string
-}
-
-func (t plainTool) Name() string        { return t.name }
-func (t plainTool) Description() string { return "plain" }
-func (t plainTool) Parameters() Schema  { return Schema{Type: "object"} }
-func (t plainTool) Safety() Safety      { return readOnlySafety("plain") }
-func (t plainTool) Run(context.Context, map[string]any) Result {
-	return okResult("ok")
-}
-
-func TestCapabilitiesOfDefaultsUnknown(t *testing.T) {
-	caps := CapabilitiesOf(plainTool{name: "mystery"})
-	if caps.Effect != EffectUnknown {
-		t.Fatalf("Effect = %v, want Unknown", caps.Effect)
-	}
-	if caps.ThreadSafe {
-		t.Fatal("ThreadSafe must be false for unclassified tools")
-	}
-}
-
-func TestCapabilitiesOfNilTool(t *testing.T) {
-	caps := CapabilitiesOf(nil)
-	if caps.Effect != EffectUnknown || caps.ThreadSafe {
-		t.Fatalf("nil tool = %+v, want fail-closed unknown", caps)
-	}
-}
-
-func TestUnknownNotThreadSafeEvenIfProviderLies(t *testing.T) {
-	tool := baseTool{
-		name: "liar",
-		capabilities: ToolCapabilities{
-			Effect:     EffectUnknown,
-			ThreadSafe: true, // invalid — Capabilities() must clear it
-		},
-	}
-	caps := tool.Capabilities()
-	if caps.ThreadSafe {
-		t.Fatal("Capabilities() must force ThreadSafe=false for EffectUnknown")
-	}
-}
-
-func TestNormalizeClearsThreadSafeOnMutators(t *testing.T) {
-	tool := baseTool{
-		name: "writer",
-		capabilities: ToolCapabilities{
-			Effect:     EffectWorkspaceWrite,
-			ThreadSafe: true, // must be cleared on read
-		},
-	}
-	if tool.Capabilities().ThreadSafe {
-		t.Fatal("WorkspaceWrite must never surface ThreadSafe=true via Capabilities()")
-	}
-	// Wrap with Run so CapabilitiesOf's CapabilityProvider path is exercised.
-	wrapped := deferredCapableTool{baseTool: tool}
-	if CapabilitiesOf(wrapped).ThreadSafe {
-		t.Fatal("CapabilitiesOf must normalize mutator ThreadSafe away")
-	}
-}
-
-func TestValidateRejectsThreadSafeUnknown(t *testing.T) {
-	problems := ValidateCapabilities("x", ToolCapabilities{Effect: EffectUnknown, ThreadSafe: true})
-	if len(problems) == 0 {
-		t.Fatal("expected validation problems for ThreadSafe+Unknown")
-	}
-}
-
-func TestValidateRejectsThreadSafeMutators(t *testing.T) {
-	for _, effect := range []EffectClass{EffectWorkspaceWrite, EffectExternalWrite, EffectInteractive, EffectProcessRead} {
-		problems := ValidateCapabilities("m", ToolCapabilities{Effect: effect, ThreadSafe: true})
-		if len(problems) == 0 {
-			t.Fatalf("effect %s: expected ThreadSafe rejection", effect)
-		}
-	}
-}
-
-func TestEveryBuiltinHasExplicitMetadata(t *testing.T) {
-	problems := ValidateBuiltinCatalog(t.TempDir())
-	if len(problems) > 0 {
-		t.Fatalf("builtin metadata coverage failed:\n  %s", strings.Join(problems, "\n  "))
-	}
-}
-
-func TestBuiltinCatalogNoUnknown(t *testing.T) {
-	counts := map[EffectClass]int{}
-	for _, tool := range BuiltinCatalog(t.TempDir()) {
-		caps := CapabilitiesOf(tool)
-		if caps.Effect == EffectUnknown {
-			t.Errorf("tool %q is EffectUnknown", tool.Name())
-		}
-		counts[caps.Effect]++
-	}
-	if counts[EffectUnknown] != 0 {
-		t.Fatalf("Unknown built-ins = %d, want 0", counts[EffectUnknown])
-	}
-	t.Logf("classification counts: %+v", counts)
-}
-
-func TestRegistryCapabilitiesLookup(t *testing.T) {
-	reg := NewRegistry()
-	reg.Register(NewScopedReadFileTool(t.TempDir(), nil))
-	caps := reg.Capabilities("read_file")
-	if caps.Effect != EffectReadOnly {
-		t.Fatalf("read_file effect = %v, want ReadOnly", caps.Effect)
-	}
-	// Missing tool → fail-closed.
-	missing := reg.Capabilities("does_not_exist")
-	if missing.Effect != EffectUnknown || missing.ThreadSafe {
-		t.Fatalf("missing tool = %+v", missing)
-	}
-}
-
-func TestFileResourceKeysNormalized(t *testing.T) {
-	keys := fileResourceKeys(map[string]any{"path": `./src/../src/main.go`})
-	if len(keys) != 1 {
-		t.Fatalf("keys = %v", keys)
-	}
-	want := ResourceKeyFile + NormalizeResourcePath("./src/../src/main.go")
-	if keys[0] != want {
-		t.Fatalf("key = %q, want %q", keys[0], want)
-	}
-	// Missing args → nil, no panic.
-	if fileResourceKeys(nil) != nil {
-		t.Fatal("nil args should yield nil keys")
-	}
-	if fileResourceKeys(map[string]any{}) != nil {
-		t.Fatal("empty args should yield nil keys")
-	}
-}
-
-func TestResourcePathNormalizePlatform(t *testing.T) {
-	// Unix-style clean
-	got := NormalizeResourcePath("a//b/../c")
-	if got != "a/c" && got != filepath.ToSlash(filepath.Clean("a//b/../c")) {
-		// After ToSlash clean should be a/c
-		if NormalizeResourcePath("a//b/../c") != "a/c" {
-			t.Fatalf("unix clean = %q", got)
-		}
-	}
-	if runtime.GOOS == "windows" {
-		// Case-insensitive
-		if NormalizeResourcePath(`C:\Foo\Bar`) != NormalizeResourcePath(`c:\foo\bar`) {
-			t.Fatalf("windows case fold failed: %q vs %q",
-				NormalizeResourcePath(`C:\Foo\Bar`), NormalizeResourcePath(`c:\foo\bar`))
-		}
-	}
-	// Empty / URL-shaped
-	if NormalizeResourcePath("") != "" {
-		t.Fatal("empty path")
-	}
-	if NormalizeResourcePath("https://user:pass@host/path") != "" {
-		t.Fatal("URL must not become a file resource path")
-	}
-}
-
-func TestEndpointResourceKeysStripSecrets(t *testing.T) {
-	keys := endpointResourceKeys(map[string]any{
-		"url": "https://user:[email protected]:443/v1?token=abc",
-	})
-	if len(keys) != 1 {
-		t.Fatalf("keys = %v", keys)
-	}
-	if strings.Contains(keys[0], "secret") || strings.Contains(keys[0], "token") || strings.Contains(keys[0], "user") {
-		t.Fatalf("endpoint key leaked secret material: %q", keys[0])
-	}
-	if !strings.Contains(keys[0], "api.example.com") {
-		t.Fatalf("endpoint key missing host: %q", keys[0])
-	}
-}
-
-func TestEndpointResourceKeysAtInQueryDoesNotStealHost(t *testing.T) {
-	// Path/query must be stripped BEFORE userinfo so ?x=@bad cannot become the host.
-	keys := endpointResourceKeys(map[string]any{
-		"url": "https://api.example.com/v1?token=@secret",
-	})
-	if len(keys) != 1 || keys[0] != ResourceKeyEndpoint+"api.example.com" {
-		t.Fatalf("keys = %v, want endpoint:api.example.com", keys)
-	}
-}
-
-func TestProcessResourceKeysNumericSessionID(t *testing.T) {
-	// write_stdin declares session_id as integer; JSON decodes as float64.
-	keys := processResourceKeys(map[string]any{"session_id": float64(42)})
-	if len(keys) != 1 || keys[0] != ResourceKeyProcess+"42" {
-		t.Fatalf("keys = %v, want process:42", keys)
-	}
-}
-
-func TestProcessResourceKeysSessionArg(t *testing.T) {
-	// terminal_session requires "session", not session_id.
-	keys := processResourceKeys(map[string]any{"session": "term-7"})
-	if len(keys) != 1 || keys[0] != ResourceKeyProcess+"term-7" {
-		t.Fatalf("keys = %v, want process:term-7", keys)
-	}
-}
-
-func TestApplyPatchResourceKeysFromDiffAndCwd(t *testing.T) {
-	patch := "--- a/old.go\n+++ b/new.go\n@@ -1 +1 @@\n-x\n+y\n"
-	keys := applyPatchResourceKeys(map[string]any{
-		"patch": patch,
-		"cwd":   "pkg",
-	})
-	// Diff paths must be joined under cwd so the same basename under different
-	// cwds does not collide for the future conflict planner.
-	want := map[string]bool{
-		ResourceKeyDirectory + "pkg":   true,
-		ResourceKeyFile + "pkg/old.go": true,
-		ResourceKeyFile + "pkg/new.go": true,
-	}
-	for _, k := range keys {
-		if !want[k] {
-			t.Fatalf("unexpected key %q in %v", k, keys)
-		}
-		delete(want, k)
-	}
-	if len(want) != 0 {
-		t.Fatalf("missing keys %v from %v", want, keys)
-	}
-}
-
-func TestDeclaredMutatorThreadSafeIsReportedByCatalogValidation(t *testing.T) {
-	// Runtime CapabilitiesOf clears ThreadSafe on mutators; catalog validation
-	// must still see the raw declaration so miswired constructors fail the gate.
-	tool := deferredCapableTool{baseTool: baseTool{
-		name: "bad_writer",
-		capabilities: ToolCapabilities{
-			Effect:     EffectWorkspaceWrite,
-			ThreadSafe: true,
-		},
-	}}
-	declared := declaredCapabilitiesOf(tool)
-	if !declared.ThreadSafe || declared.Effect != EffectWorkspaceWrite {
-		t.Fatalf("declared = %+v, want raw WorkspaceWrite+ThreadSafe", declared)
-	}
-	problems := ValidateCapabilities(tool.Name(), declared)
-	if len(problems) == 0 {
-		t.Fatal("expected ValidateCapabilities to reject raw WorkspaceWrite+ThreadSafe")
-	}
-	// Runtime view stays safe.
-	if CapabilitiesOf(tool).ThreadSafe {
-		t.Fatal("runtime CapabilitiesOf must still clear ThreadSafe")
-	}
-}
-
-func TestApplyPatchResourceKeysFallbackWorkspace(t *testing.T) {
-	keys := applyPatchResourceKeys(map[string]any{"patch": "not a real diff"})
-	if len(keys) != 1 || keys[0] != ResourceKeyWorkspace+"root" {
-		t.Fatalf("keys = %v, want workspace:root fallback", keys)
-	}
-}
-
-func TestWorkspaceWriteClassifications(t *testing.T) {
-	root := t.TempDir()
-	for _, tool := range []Tool{
-		NewScopedWriteFileTool(root, nil),
-		NewScopedEditFileTool(root, nil),
-		NewScopedApplyPatchTool(root, nil),
-		NewScopedBashTool(root, nil),
-	} {
-		if got := CapabilitiesOf(tool).Effect; got != EffectWorkspaceWrite {
-			t.Errorf("%s effect = %v, want WorkspaceWrite", tool.Name(), got)
-		}
-		if CapabilitiesOf(tool).ThreadSafe {
-			t.Errorf("%s must not be ThreadSafe", tool.Name())
-		}
-	}
-}
-
-func TestInteractiveClassifications(t *testing.T) {
-	root := t.TempDir()
-	for _, name := range []string{"ask_user", "exec_command", "write_stdin", "update_plan", "escalate_model", "request_permissions", "browser_open", "terminal_session", "desktop_action"} {
-		var found Tool
-		for _, tool := range BuiltinCatalog(root) {
-			if tool.Name() == name {
-				found = tool
-				break
-			}
-		}
-		if found == nil {
-			t.Fatalf("tool %q not in catalog", name)
-		}
-		if got := CapabilitiesOf(found).Effect; got != EffectInteractive {
-			t.Errorf("%s effect = %v, want Interactive", name, got)
-		}
-	}
-}
-
-func TestReadOnlyClassifications(t *testing.T) {
-	root := t.TempDir()
-	for _, name := range []string{"read_file", "read_minified_file", "list_directory", "glob", "grep", "lsp_navigate", "skill", "web_fetch", "web_search", "tool_search"} {
-		var found Tool
-		for _, tool := range BuiltinCatalog(root) {
-			if tool.Name() == name {
-				found = tool
-				break
-			}
-		}
-		if found == nil {
-			t.Fatalf("tool %q not in catalog", name)
-		}
-		caps := CapabilitiesOf(found)
-		if caps.Effect != EffectReadOnly {
-			t.Errorf("%s effect = %v, want ReadOnly", name, caps.Effect)
-		}
-		// Current catalog intentionally keeps read tools non-thread-safe
-		// until each is audited for shared mutable state (FileTracker, etc.).
-		if caps.ThreadSafe {
-			t.Errorf("%s ThreadSafe=true without explicit audit", name)
-		}
-	}
-}
-
-func TestValidateBuiltinCatalogRejectsUnknownRaw(t *testing.T) {
-	// Prove the gate is not a rubber stamp: raw Unknown+ThreadSafe is rejected.
-	problems := ValidateCapabilities("ghost", ToolCapabilities{Effect: EffectUnknown, ThreadSafe: true})
-	if len(problems) == 0 {
-		t.Fatal("expected ValidateCapabilities to reject Unknown+ThreadSafe")
-	}
-}
-
-func TestCaptureArtifactIsWorkspaceWrite(t *testing.T) {
-	for _, tool := range NewLocalControlArtifactTools(LocalControlArtifactOptions{}) {
-		if CapabilitiesOf(tool).Effect != EffectWorkspaceWrite {
-			t.Fatalf("%s effect = %v, want WorkspaceWrite", tool.Name(), CapabilitiesOf(tool).Effect)
-		}
-	}
-}
-
-func TestCapabilitiesLookupRace(t *testing.T) {
-	reg := NewRegistry()
-	for _, tool := range BuiltinCatalog(t.TempDir()) {
-		reg.Register(tool)
-	}
-	var wg sync.WaitGroup
-	for i := 0; i < 32; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
-			for _, tool := range reg.All() {
-				_ = reg.Capabilities(tool.Name())
-				_ = CapabilitiesOf(tool)
-				caps := CapabilitiesOf(tool)
-				if caps.ResourceKeys != nil {
-					_ = caps.ResourceKeys(map[string]any{"path": "a/b.go", "url": "https://example.com"})
-				}
-			}
-		}()
-	}
-	wg.Wait()
-}
-
-func TestEffectStringAndValid(t *testing.T) {
-	for _, e := range []EffectClass{EffectUnknown, EffectReadOnly, EffectProcessRead, EffectWorkspaceWrite, EffectExternalWrite, EffectInteractive} {
-		if !e.Valid() {
-			t.Fatalf("%v should be valid", e)
-		}
-		if e.String() == "invalid" || e.String() == "" {
-			t.Fatalf("String for %d", e)
-		}
-	}
-	if EffectClass(99).Valid() {
-		t.Fatal("invalid class reported valid")
-	}
-}
-
-// deferredCapableTool models a deferred-eligible built-in that still carries
-// explicit capabilities (MCP tools do not implement CapabilityProvider and
-// stay Unknown by design).
-type deferredCapableTool struct {
-	baseTool
-}
-
-func (t deferredCapableTool) Deferred() bool { return true }
-
-func (t deferredCapableTool) Run(context.Context, map[string]any) Result {
-	return okResult("ok")
-}
-
-func TestDeferredToolPreservesCapabilities(t *testing.T) {
-	tool := deferredCapableTool{baseTool: baseTool{
-		name: "deferred_reader",
-		capabilities: ToolCapabilities{
-			Effect:       EffectReadOnly,
-			ThreadSafe:   false,
-			ResourceKeys: fileResourceKeys,
-		},
-	}}
-	if !IsDeferred(tool) {
-		t.Fatal("expected deferred")
-	}
-	reg := NewRegistry()
-	reg.Register(tool)
-	got, ok := reg.Get("deferred_reader")
-	if !ok {
-		t.Fatal("missing registration")
-	}
-	caps := CapabilitiesOf(got)
-	if caps.Effect != EffectReadOnly {
-		t.Fatalf("effect = %v after deferred register", caps.Effect)
-	}
-	keys := caps.ResourceKeys(map[string]any{"path": "pkg/a.go"})
-	if len(keys) != 1 || keys[0] != ResourceKeyFile+NormalizeResourcePath("pkg/a.go") {
-		t.Fatalf("resource keys = %v", keys)
-	}
-}
-
-func TestPluginAndMCPStyleDefaultUnknown(t *testing.T) {
-	// Plugin and MCP adapters do not implement CapabilityProvider; they must
-	// remain fail-closed (serialized) until they opt into a trusted contract.
-	for _, name := range []string{"mcp_github_create_issue", "plugin_custom_tool"} {
-		caps := CapabilitiesOf(plainTool{name: name})
-		if caps.Effect != EffectUnknown || caps.ThreadSafe {
-			t.Fatalf("%s = %+v, want Unknown/not-thread-safe", name, caps)
-		}
-	}
-}
-
-func TestSchemasUnchangedForCoreTools(t *testing.T) {
-	// Metadata must not alter tool names or parameter shapes.
-	root := t.TempDir()
-	read := NewScopedReadFileTool(root, nil)
-	if read.Name() != "read_file" {
-		t.Fatalf("name = %q", read.Name())
-	}
-	params := read.Parameters()
-	if params.Type != "object" {
-		t.Fatalf("params type = %q", params.Type)
-	}
-	if _, ok := params.Properties["path"]; !ok {
-		t.Fatal("path property missing")
-	}
-	// Safety path unchanged.
-	if read.Safety().SideEffect != SideEffectRead {
-		t.Fatalf("safety side effect changed: %v", read.Safety().SideEffect)
-	}
-}

internal/tools/edit_file.go

@@ -34,8 +34,7 @@ func NewScopedEditFileTool(workspaceRoot string, scope PathScope) Tool {
 				Required:             []string{"path", "old_string", "new_string"},
 				AdditionalProperties: false,
 			},
-			safety:       promptSafety(SideEffectWrite, "Edits files in place."),
-			capabilities: ToolCapabilities{Effect: EffectWorkspaceWrite, ThreadSafe: false, ResourceKeys: fileResourceKeys},
+			safety: promptSafety(SideEffectWrite, "Edits files in place."),
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/escalate_model.go

@@ -57,8 +57,6 @@ func newEscalateModelTool(registry modelregistry.Registry) escalateModelTool {
 				Reason:          "Requests a mid-run switch to a stronger model; the agent loop performs the switch.",
 				AdvertiseInAuto: true,
 			},
-			// Requests mid-run model switch; session-bound.
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false},
 		},
 		registry: registry,
 	}

internal/tools/exec_command.go

@@ -465,8 +465,6 @@ func NewScopedExecCommandTool(workspaceRoot string, scope PathScope, manager *ex
 				AdditionalProperties: false,
 			},
 			safety: promptSafety(SideEffectShell, "Shell commands can read, write, or execute programs."),
-			// PTY/shell session — never concurrent.
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: processResourceKeys},
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,
@@ -719,8 +717,6 @@ func NewWriteStdinTool(manager *execSessionManager) Tool {
 				Reason:          "Sending stdin can drive an existing shell process beyond the original command; empty polling and Ctrl-C interrupts are allowed automatically.",
 				AdvertiseInAuto: true,
 			},
-			// Writes to a retained process stdin — process interaction.
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: processResourceKeys},
 		},
 		manager: manager,
 	}

internal/tools/glob.go

@@ -38,8 +38,7 @@ func NewScopedGlobTool(workspaceRoot string, scope PathScope) Tool {
 				Required:             []string{"pattern"},
 				AdditionalProperties: false,
 			},
-			safety:       readOnlySafety("Finds matching paths without reading contents or modifying files."),
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: false, ResourceKeys: workspaceResourceKeys},
+			safety: readOnlySafety("Finds matching paths without reading contents or modifying files."),
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/grep.go

@@ -53,8 +53,7 @@ func NewScopedGrepTool(workspaceRoot string, scope PathScope) Tool {
 				Required:             []string{"pattern"},
 				AdditionalProperties: false,
 			},
-			safety:       readOnlySafety("Searches file paths and matching lines without modifying files."),
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: false, ResourceKeys: workspaceResourceKeys},
+			safety: readOnlySafety("Searches file paths and matching lines without modifying files."),
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/list_directory.go

@@ -33,8 +33,7 @@ func NewScopedListDirectoryTool(workspaceRoot string, scope PathScope) Tool {
 				},
 				AdditionalProperties: false,
 			},
-			safety:       readOnlySafety("Lists directory entries without modifying files."),
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: false, ResourceKeys: directoryResourceKeys},
+			safety: readOnlySafety("Lists directory entries without modifying files."),
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,

internal/tools/local_browser.go

@@ -53,8 +53,7 @@ func newBrowserInstallTool(options localcontrol.BrowserOptions) Tool {
 				},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Downloads browser runtime files for local browser automation."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Downloads browser runtime files for local browser automation."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -99,8 +98,7 @@ func newBrowserLaunchToolWithLauncher(options localcontrol.BrowserOptions, launc
 				Required:             []string{"app"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Launches a supported local Chromium/Electron app with DevTools enabled and attaches local browser automation."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Launches a supported local Chromium/Electron app with DevTools enabled and attaches local browser automation."),
 		},
 		browser:  localcontrol.NewBrowser(options),
 		launcher: launcher,
@@ -174,8 +172,7 @@ func newBrowserConnectTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"target"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Attaches to a local browser or Electron app exposed over the Chrome DevTools Protocol."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Attaches to a local browser or Electron app exposed over the Chrome DevTools Protocol."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -214,8 +211,7 @@ func newBrowserOpenTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"url"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectNetwork, "Opens a URL through the local browser automation helper. For already-running Chrome/Electron apps, use browser_connect instead. If first use reports a missing browser runtime, run browser_install once."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectNetwork, "Opens a URL through the local browser automation helper. For already-running Chrome/Electron apps, use browser_connect instead. If first use reports a missing browser runtime, run browser_install once."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -257,8 +253,7 @@ func newBrowserSnapshotTool(options localcontrol.BrowserOptions) Tool {
 				},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Reads the current local browser automation session state."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Reads the current local browser automation session state."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -297,8 +292,7 @@ func newBrowserClickTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"ref"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Clicks an element in the local browser automation session."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Clicks an element in the local browser automation session."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -338,8 +332,7 @@ func newBrowserTypeTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"ref", "text"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Types text into an element in the local browser automation session."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Types text into an element in the local browser automation session."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -378,8 +371,7 @@ func newBrowserPressTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"key"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Presses a key in the local browser automation session."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Presses a key in the local browser automation session."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}
@@ -464,8 +456,7 @@ func newBrowserActionTool(options localcontrol.BrowserOptions) Tool {
 				Required:             []string{"command"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalBrowser, "Interacts with the local browser automation session."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: sessionResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalBrowser, "Interacts with the local browser automation session."),
 		},
 		browser: localcontrol.NewBrowser(options),
 	}

internal/tools/local_capture.go

@@ -58,8 +58,6 @@ func newCaptureArtifactTool(options LocalControlArtifactOptions) Tool {
 				AdditionalProperties: false,
 			},
 			safety: localControlSafety(enabled, SideEffectLocalControl, "Captures local browser, desktop, or terminal state into the configured artifact directory."),
-			// Captures write artifacts under the configured directory.
-			capabilities: ToolCapabilities{Effect: EffectWorkspaceWrite, ThreadSafe: false},
 		},
 		browser:      localcontrol.NewBrowser(options.Browser),
 		desktop:      localcontrol.NewDesktop(options.Desktop),

internal/tools/local_desktop.go

@@ -35,8 +35,7 @@ func newDesktopWindowsTool(options localcontrol.DesktopOptions) Tool {
 				},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalDesktop, "Reads native desktop window metadata."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false},
+			safety: localControlSafety(options.Enabled, SideEffectLocalDesktop, "Reads native desktop window metadata."),
 		},
 		desktop: localcontrol.NewDesktop(options),
 	}
@@ -82,8 +81,7 @@ func newDesktopSnapshotTool(options localcontrol.DesktopOptions) Tool {
 				Required:             []string{"pid", "window_id"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalDesktop, "Reads a native desktop window state."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false},
+			safety: localControlSafety(options.Enabled, SideEffectLocalDesktop, "Reads a native desktop window state."),
 		},
 		desktop: localcontrol.NewDesktop(options),
 	}
@@ -127,8 +125,7 @@ func newDesktopActionTool(options localcontrol.DesktopOptions) Tool {
 				Required:             []string{"command", "input"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalDesktop, "Interacts with native desktop windows."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false},
+			safety: localControlSafety(options.Enabled, SideEffectLocalDesktop, "Interacts with native desktop windows."),
 		},
 		desktop: localcontrol.NewDesktop(options),
 	}

internal/tools/local_terminal.go

@@ -45,8 +45,7 @@ func newTerminalSessionTool(options localcontrol.TerminalOptions) Tool {
 				Required:             []string{"action", "session"},
 				AdditionalProperties: false,
 			},
-			safety:       localControlSafety(options.Enabled, SideEffectLocalTerminal, "Launches or interacts with a local virtual terminal session."),
-			capabilities: ToolCapabilities{Effect: EffectInteractive, ThreadSafe: false, ResourceKeys: processResourceKeys},
+			safety: localControlSafety(options.Enabled, SideEffectLocalTerminal, "Launches or interacts with a local virtual terminal session."),
 		},
 		terminal: localcontrol.NewTerminal(options),
 	}

internal/tools/lsp_navigate.go

@@ -58,8 +58,7 @@ func NewScopedLSPNavigateTool(workspaceRoot string, scope PathScope) Tool {
 				Required:             []string{"op", "path"},
 				AdditionalProperties: false,
 			},
-			safety:       readOnlySafety("Queries the language server for code navigation; reads files, modifies nothing."),
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: false, ResourceKeys: fileResourceKeys},
+			safety: readOnlySafety("Queries the language server for code navigation; reads files, modifies nothing."),
 		},
 		workspaceRoot: root,
 		scope:         scope,

internal/tools/read_file.go

@@ -39,8 +39,6 @@ func NewScopedReadFileTool(workspaceRoot string, scope PathScope) Tool {
 				AdditionalProperties: false,
 			},
 			safety: readOnlySafety("Reads file contents without modifying files."),
-			// ThreadSafe=false: may update FileTracker session state on read.
-			capabilities: ToolCapabilities{Effect: EffectReadOnly, ThreadSafe: false, ResourceKeys: fileResourceKeys},
 		},
 		workspaceRoot: normalizeWorkspaceRoot(workspaceRoot),
 		scope:         scope,