Merge remote-tracking branch 'origin/main' into test/clear-fetch-disable-env

The variable turns off both the models.dev fetch and the cached overlay,
and four tests in modelsdev_test.go inherited it from the shell. With it
set, TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering and
TestDefaultModelEntriesAppliesFreshCache fail, and
TestCachedModelsDevProvidersIgnoresStaleCache and
TestCachedModelsDevProvidersRequiresOptIn pass without reaching the
check they are named for: the nil they expect comes from the variable.

Clear it in each of them, as the two sibling refresh tests already do.

docs/EXTENDING.md

@@ -110,8 +110,6 @@ Discovery roots (earlier wins on name collisions):
 
 A missing directory is fine — Zero just omits it. Management commands (`zero skill add` / `zero skill remove`) always write to the primary Zero dir only; `list` / `info` search primary + `~/.agents/skills`. `skill info` reports lock source/hash when present and whether the on-disk `SKILL.md` has drifted from the recorded hash.
 
-For a skill installed since Zero started installing bundled files, `skill info` also reports drift in any file installed with it.
-
 ```text
 ~/.local/share/zero/skills/          # primary (install/remove target)
   run-benchmarks/
@@ -142,15 +140,6 @@ Only `name` and `description` are recognized in the frontmatter today. The `name
 
 The `skill` tool lets the agent load any discovered skill by name (primary, agents, or plugin).
 
-A skill can ship files beside its `SKILL.md`, in the [agentskills.io](https://agentskills.io/specification) layout: `scripts/`, `references/`, `assets/` or anything else the instructions refer to. `zero skill add` installs the whole skill directory, leaving out hidden entries such as `.git` and not following or copying symlinks (it lists any it skipped). A skill larger than 1,000 files or 32 MiB is refused. When the `skill` tool loads a skill that has such files, its output ends with the skill's directory and a list of the files, so instructions like "run `scripts/fill.py`" can be followed. Running a bundled script goes through the normal tool permissions; installing or loading a skill never runs anything.
-
-```text
-pdf/
-  SKILL.md
-  scripts/fill.py
-  references/form-fields.md
-```
-
 ## 4. Hooks
 
 Hooks fire shell commands on lifecycle events. Configure them in JSON:

internal/cli/distribution.go

@@ -92,25 +92,13 @@ func runSkillAdd(args []string, dir string, stdout io.Writer, stderr io.Writer)
 	lines := []string{}
 	if result.Updated {
 		lines = append(lines, fmt.Sprintf("Updated skill %q.", result.Name))
-		switch {
-		case result.PreviousHash != result.Hash:
-			lines = append(lines, fmt.Sprintf("  hash: %s -> %s", shortHash(result.PreviousHash), shortHash(result.Hash)))
-		case result.PreviousTreeHash == "":
-			// The install this replaced predates tree hashes and copied SKILL.md
-			// alone, so the bundled files are new rather than changed.
-			lines = append(lines, "  SKILL.md is unchanged; the files bundled with it are now installed too")
-		default:
-			lines = append(lines, "  SKILL.md is unchanged; the files bundled with it changed")
-		}
+		lines = append(lines, fmt.Sprintf("  hash: %s -> %s", shortHash(result.PreviousHash), shortHash(result.Hash)))
 	} else {
 		lines = append(lines, fmt.Sprintf("Installed skill %q.", result.Name))
 		lines = append(lines, "  hash: "+result.Hash)
 	}
 	lines = append(lines, "  source: "+result.Source)
 	lines = append(lines, "  path: "+result.Path)
-	if len(result.SkippedLinks) > 0 {
-		lines = append(lines, "  not copied (symlinks): "+strings.Join(result.SkippedLinks, ", "))
-	}
 	if _, err := fmt.Fprintln(stdout, redaction.RedactString(strings.Join(lines, "\n"), redaction.Options{})); err != nil {
 		return exitCrash
 	}

internal/cli/distribution_test.go

@@ -7,8 +7,6 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
-
-	"github.com/Twigpine/zero/internal/skills"
 )
 
 func writeSourceSkillDir(t *testing.T, dir string, content string) string {
@@ -393,133 +391,3 @@ func TestRunDistributionCommandsDoNotLaunchTUI(t *testing.T) {
 		}
 	}
 }
-
-// A reinstall that only changed a bundled script must still read as an update
-// (#584): the SKILL.md hash is the same, so "hash: X -> X" would say nothing.
-func TestRunSkillAddReportsAnUpdateToBundledFilesOnly(t *testing.T) {
-	skillsDir := t.TempDir()
-	src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
-		"---\nname: pdf\ndescription: Fill forms.\n---\nRun scripts/fill.py.\n")
-	script := filepath.Join(src, "scripts", "fill.py")
-	if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(script, []byte("print('v1')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	deps := appDeps{skillsDir: func() string { return skillsDir }}
-
-	var stdout, stderr bytes.Buffer
-	if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
-		t.Fatalf("first skill add exit = %d, stderr = %s", exit, stderr.String())
-	}
-	if err := os.WriteFile(script, []byte("print('v2')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	stdout.Reset()
-	stderr.Reset()
-	if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
-		t.Fatalf("second skill add exit = %d, stderr = %s", exit, stderr.String())
-	}
-	for _, want := range []string{`Updated skill "pdf".`, "SKILL.md is unchanged; the files bundled with it changed"} {
-		if !strings.Contains(stdout.String(), want) {
-			t.Fatalf("second add output does not say %q:\n%s", want, stdout.String())
-		}
-	}
-}
-
-// An install from before tree hashes copied SKILL.md alone and recorded no tree
-// hash. Adding the skill again over that entry must not claim bundled files
-// changed: there are none, or they are installed for the first time.
-func TestRunSkillAddOverAnEntryWithoutATreeHash(t *testing.T) {
-	for _, tc := range []struct {
-		name    string
-		script  bool
-		want    []string
-		notWant []string
-	}{
-		{
-			name:    "SKILL.md only",
-			want:    []string{`Installed skill "pdf".`},
-			notWant: []string{"Updated skill", "files bundled with it"},
-		},
-		{
-			name:    "with a bundled script",
-			script:  true,
-			want:    []string{`Updated skill "pdf".`, "SKILL.md is unchanged; the files bundled with it are now installed too"},
-			notWant: []string{"files bundled with it changed"},
-		},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			skillsDir := t.TempDir()
-			src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
-				"---\nname: pdf\ndescription: Fill forms.\n---\nRun scripts/fill.py.\n")
-			if tc.script {
-				script := filepath.Join(src, "scripts", "fill.py")
-				if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
-					t.Fatal(err)
-				}
-				if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
-					t.Fatal(err)
-				}
-			}
-			deps := appDeps{skillsDir: func() string { return skillsDir }}
-
-			var stdout, stderr bytes.Buffer
-			if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
-				t.Fatalf("first skill add exit = %d, stderr = %s", exit, stderr.String())
-			}
-			lock, err := skills.ReadLock(skillsDir)
-			if err != nil {
-				t.Fatal(err)
-			}
-			entry := lock["pdf"]
-			entry.TreeHash = ""
-			lock["pdf"] = entry
-			data, err := json.Marshal(lock)
-			if err != nil {
-				t.Fatal(err)
-			}
-			if err := os.WriteFile(filepath.Join(skillsDir, skills.LockFileName), data, 0o644); err != nil {
-				t.Fatal(err)
-			}
-
-			stdout.Reset()
-			stderr.Reset()
-			if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
-				t.Fatalf("second skill add exit = %d, stderr = %s", exit, stderr.String())
-			}
-			for _, want := range tc.want {
-				if !strings.Contains(stdout.String(), want) {
-					t.Fatalf("second add output does not say %q:\n%s", want, stdout.String())
-				}
-			}
-			for _, notWant := range tc.notWant {
-				if strings.Contains(stdout.String(), notWant) {
-					t.Fatalf("second add output says %q:\n%s", notWant, stdout.String())
-				}
-			}
-		})
-	}
-}
-
-func TestRunSkillAddListsTheSymlinksItDidNotCopy(t *testing.T) {
-	skillsDir := t.TempDir()
-	src := writeSourceSkillDir(t, filepath.Join(t.TempDir(), "src"),
-		"---\nname: pdf\ndescription: Fill forms.\n---\nbody\n")
-	outside := filepath.Join(t.TempDir(), "notes.md")
-	if err := os.WriteFile(outside, []byte("outside\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Symlink(outside, filepath.Join(src, "notes.md")); err != nil {
-		t.Skipf("symlinks unavailable: %v", err)
-	}
-	var stdout, stderr bytes.Buffer
-	deps := appDeps{skillsDir: func() string { return skillsDir }}
-	if exit := runWithDeps([]string{"skill", "add", src}, &stdout, &stderr, deps); exit != 0 {
-		t.Fatalf("skill add exit = %d, stderr = %s", exit, stderr.String())
-	}
-	if !strings.Contains(stdout.String(), "not copied (symlinks): notes.md") {
-		t.Fatalf("add output does not list the skipped symlink:\n%s", stdout.String())
-	}
-}

internal/oauth/encrypt.go

@@ -0,0 +1,191 @@
+package oauth
+
+import (
+	"crypto/aes"
+	"crypto/cipher"
+	"crypto/rand"
+	"errors"
+	"fmt"
+	"io"
+	"os"
+	"path/filepath"
+	"time"
+)
+
+const (
+	// secretBytes is the AES-256 key length kept in the per-user secret file.
+	secretBytes = 32
+
+	secretRetryAttempts = 500
+	secretRetryDelay    = 2 * time.Millisecond
+)
+
+// aesGCMCrypter encrypts the token file at rest with AES-256-GCM under a
+// per-user random secret persisted (0600) beside the token file. The on-disk
+// blob is nonce || ciphertext; GCM provides confidentiality AND tamper
+// detection, so a corrupted/forged file fails closed on open. This is the
+// opt-in "encrypted-file" storage backend; the default backend writes the
+// 0600 plaintext JSON unchanged.
+type aesGCMCrypter struct {
+	secretPath string
+}
+
+func newAESGCMCrypter(secretPath string) *aesGCMCrypter {
+	return &aesGCMCrypter{secretPath: secretPath}
+}
+
+// aead loads (or, when create is set, generates) the secret and returns the GCM
+// AEAD. open passes create=false so a missing secret is a hard error rather than
+// silently minting a new key that could never decrypt the existing file.
+func (c *aesGCMCrypter) aead(create bool) (cipher.AEAD, error) {
+	secret, err := loadOrCreateSecret(c.secretPath, create)
+	if err != nil {
+		return nil, err
+	}
+	block, err := aes.NewCipher(secret)
+	if err != nil {
+		return nil, fmt.Errorf("oauth: build cipher: %w", err)
+	}
+	return cipher.NewGCM(block)
+}
+
+// seal encrypts plaintext, prefixing a fresh random nonce. It creates the secret
+// on first use.
+func (c *aesGCMCrypter) seal(plaintext []byte) ([]byte, error) {
+	gcm, err := c.aead(true)
+	if err != nil {
+		return nil, err
+	}
+	nonce := make([]byte, gcm.NonceSize())
+	if _, err := io.ReadFull(rand.Reader, nonce); err != nil {
+		return nil, fmt.Errorf("oauth: generate nonce: %w", err)
+	}
+	return gcm.Seal(nonce, nonce, plaintext, nil), nil
+}
+
+// open decrypts a nonce||ciphertext blob, failing closed on a missing secret,
+// a short blob, or a failed authentication tag (tampering / wrong key).
+func (c *aesGCMCrypter) open(blob []byte) ([]byte, error) {
+	gcm, err := c.aead(false)
+	if err != nil {
+		return nil, err
+	}
+	if len(blob) < gcm.NonceSize() {
+		return nil, errors.New("oauth: encrypted token file is too short")
+	}
+	nonce, ciphertext := blob[:gcm.NonceSize()], blob[gcm.NonceSize():]
+	plaintext, err := gcm.Open(nil, nonce, ciphertext, nil)
+	if err != nil {
+		return nil, fmt.Errorf("oauth: decrypt token file (wrong secret or tampered): %w", err)
+	}
+	return plaintext, nil
+}
+
+// loadOrCreateSecret reads the 32-byte secret at path. When create is set and
+// the file is absent, it generates a random secret and creates the file
+// atomically (0600). A wrong-sized existing secret fails closed (corruption).
+func loadOrCreateSecret(path string, create bool) ([]byte, error) {
+	if data, err := readSecretFileRetry(path); err == nil {
+		return data, nil
+	} else if !errors.Is(err, os.ErrNotExist) {
+		return nil, err
+	}
+	if !create {
+		return nil, fmt.Errorf("oauth: token secret %s is missing; cannot decrypt the token file", path)
+	}
+	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
+		return nil, err
+	}
+	return createSecretFile(path)
+}
+
+func createSecretFile(path string) ([]byte, error) {
+	lockPath := path + ".lock"
+	var lastErr error
+	for attempt := 0; attempt < secretRetryAttempts; attempt++ {
+		lock, err := os.OpenFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
+		if err == nil {
+			_ = lock.Close()
+			defer os.Remove(lockPath)
+			if data, rerr := readSecretFileRetry(path); rerr == nil {
+				return data, nil
+			} else if !errors.Is(rerr, os.ErrNotExist) {
+				return nil, rerr
+			}
+			return writeNewSecretFile(path)
+		}
+		// On Windows a concurrent holder's os.Remove leaves the lock file in a
+		// "delete pending" state, so an O_EXCL create races it with
+		// ERROR_ACCESS_DENIED (os.ErrPermission) rather than ErrExist. Treat that
+		// as contention and retry too — mirroring acquireFileLock in lock.go —
+		// otherwise concurrent secret creation spuriously fails on Windows.
+		if !errors.Is(err, os.ErrExist) && !errors.Is(err, os.ErrPermission) {
+			return nil, fmt.Errorf("oauth: create token secret lock: %w", err)
+		}
+		// Remember the lock-creation error itself rather than a subsequent
+		// "secret file doesn't exist yet" read error -- otherwise a real
+		// contention/ACL problem is masked by the expected-while-waiting
+		// ErrNotExist once the retries are exhausted.
+		lastErr = err
+		if data, rerr := readSecretFileRetry(path); rerr == nil {
+			return data, nil
+		}
+		time.Sleep(secretRetryDelay)
+	}
+	return nil, fmt.Errorf("oauth: timed out waiting for token secret %s: %w", path, lastErr)
+}
+
+func writeNewSecretFile(path string) ([]byte, error) {
+	secret := make([]byte, secretBytes)
+	if _, err := io.ReadFull(rand.Reader, secret); err != nil {
+		return nil, fmt.Errorf("oauth: generate token secret: %w", err)
+	}
+	// The encryption key is published exactly like the token blob: through the
+	// protected per-store directory, under a random name a sandboxed process
+	// cannot guess, wait for, or rename away.
+	tmp, tmpPath, err := createPublicationFile(path)
+	if err != nil {
+		return nil, fmt.Errorf("oauth: create token secret temp file: %w", err)
+	}
+	defer os.Remove(tmpPath)
+	if _, werr := tmp.Write(secret); werr != nil {
+		_ = tmp.Close()
+		return nil, fmt.Errorf("oauth: write token secret: %w", werr)
+	}
+	if cerr := tmp.Close(); cerr != nil {
+		return nil, fmt.Errorf("oauth: write token secret: %w", cerr)
+	}
+	if err := os.Rename(tmpPath, path); err != nil {
+		return nil, fmt.Errorf("oauth: publish token secret: %w", err)
+	}
+	return secret, nil
+}
+
+func readSecretFileRetry(path string) ([]byte, error) {
+	var lastErr error
+	for attempt := 0; attempt < secretRetryAttempts; attempt++ {
+		data, err := readSecretFile(path)
+		if !isTransientSecretAccessError(err) {
+			return data, err
+		}
+		lastErr = err
+		time.Sleep(secretRetryDelay)
+	}
+	return nil, fmt.Errorf("oauth: timed out waiting for token secret %s: %w", path, lastErr)
+}
+
+// readSecretFile reads and validates the secret at path, returning a wrapped
+// os.ErrNotExist when it is absent so callers can branch on creation.
+func readSecretFile(path string) ([]byte, error) {
+	data, err := os.ReadFile(path)
+	if err != nil {
+		if errors.Is(err, os.ErrNotExist) {
+			return nil, err
+		}
+		return nil, fmt.Errorf("oauth: read token secret: %w", err)
+	}
+	if len(data) != secretBytes {
+		return nil, fmt.Errorf("oauth: token secret at %s has unexpected size %d", path, len(data))
+	}
+	return data, nil
+}

internal/oauth/encrypt_test.go

@@ -1,13 +1,103 @@
 package oauth
 
 import (
+	"bytes"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
+	"sync"
 	"testing"
-	"time"
 )
 
+func TestLoadOrCreateSecretConcurrentConverges(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "tok.json.secret")
+	const n = 16
+	secrets := make([][]byte, n)
+	errs := make([]error, n)
+	var wg sync.WaitGroup
+	for i := 0; i < n; i++ {
+		wg.Add(1)
+		go func(i int) {
+			defer wg.Done()
+			secrets[i], errs[i] = loadOrCreateSecret(path, true)
+		}(i)
+	}
+	wg.Wait()
+	// Exactly one creator wins; every racer must converge on the same on-disk
+	// secret rather than reading a half-published file or orphaning its own.
+	for i := 0; i < n; i++ {
+		if errs[i] != nil {
+			t.Fatalf("goroutine %d: %v", i, errs[i])
+		}
+		if !bytes.Equal(secrets[i], secrets[0]) {
+			t.Fatalf("goroutine %d got a divergent secret; concurrent create did not converge", i)
+		}
+	}
+}
+
+func TestAESGCMCrypterRoundTripAndTamper(t *testing.T) {
+	secretPath := filepath.Join(t.TempDir(), "tok.json.secret")
+	c := newAESGCMCrypter(secretPath)
+	plaintext := []byte(`{"schemaVersion":1,"tokens":{}}`)
+	blob, err := c.seal(plaintext)
+	if err != nil {
+		t.Fatalf("seal: %v", err)
+	}
+	if bytes.Contains(blob, plaintext) {
+		t.Fatal("sealed blob must not contain the plaintext")
+	}
+	got, err := c.open(blob)
+	if err != nil {
+		t.Fatalf("open: %v", err)
+	}
+	if !bytes.Equal(got, plaintext) {
+		t.Fatalf("round-trip mismatch: %q", got)
+	}
+	// Tamper: flipping any byte breaks the GCM tag.
+	tampered := append([]byte(nil), blob...)
+	tampered[len(tampered)-1] ^= 0xff
+	if _, err := c.open(tampered); err == nil {
+		t.Fatal("open must reject a tampered blob (GCM auth)")
+	}
+}
+
+func TestLoadOrCreateSecret(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "tok.json.secret")
+	// Missing + create=false => fail closed (can't decrypt without the secret).
+	if _, err := loadOrCreateSecret(path, false); err == nil {
+		t.Fatal("missing secret with create=false must error")
+	}
+	secret, err := loadOrCreateSecret(path, true)
+	if err != nil {
+		t.Fatalf("create: %v", err)
+	}
+	if len(secret) != secretBytes {
+		t.Fatalf("secret length = %d, want %d", len(secret), secretBytes)
+	}
+	// Stable across reads.
+	again, err := loadOrCreateSecret(path, false)
+	if err != nil || !bytes.Equal(again, secret) {
+		t.Fatalf("secret not stable: %v", err)
+	}
+	if runtime.GOOS != "windows" {
+		info, err := os.Stat(path)
+		if err != nil {
+			t.Fatalf("stat secret: %v", err)
+		}
+		if perm := info.Mode().Perm(); perm != 0o600 {
+			t.Fatalf("secret file mode = %o, want 600", perm)
+		}
+	}
+	// Wrong-sized secret fails closed.
+	if err := os.WriteFile(path, []byte("short"), 0o600); err != nil {
+		t.Fatalf("corrupt secret: %v", err)
+	}
+	if _, err := loadOrCreateSecret(path, true); err == nil {
+		t.Fatal("wrong-sized secret must error")
+	}
+}
+
 func newEncryptedStore(t *testing.T) (*Store, string) {
 	t.Helper()
 	path := filepath.Join(t.TempDir(), "oauth-tokens.json")
@@ -112,36 +202,3 @@ func TestEncryptedStoreMissingSecretFailsClosed(t *testing.T) {
 		t.Fatal("missing secret must fail closed, not return empty")
 	}
 }
-
-// TestEncryptedStoreReclaimsStaleSecretLock reproduces a crash while the
-// encrypted-file backend was creating its AES key. The key-creation lock is an
-// O_EXCL ownership marker, not a kernel lock, so a process killed between
-// creating and removing it leaves a file that every later Save waits on and
-// then rejects forever. A stale marker must be reclaimed instead.
-func TestEncryptedStoreReclaimsStaleSecretLock(t *testing.T) {
-	s, path := newEncryptedStore(t)
-	lockPath := path + ".secret.lock"
-	if err := os.WriteFile(lockPath, []byte("crashed-holder"), 0o600); err != nil {
-		t.Fatalf("write stale secret lock: %v", err)
-	}
-	stale := time.Now().Add(-2 * time.Minute)
-	if err := os.Chtimes(lockPath, stale, stale); err != nil {
-		t.Fatalf("backdate stale secret lock: %v", err)
-	}
-
-	if err := s.Save(ProviderKey("demo"), Token{AccessToken: "after-crash"}); err != nil {
-		t.Fatalf("Save must reclaim a stale secret lock, got: %v", err)
-	}
-	if _, err := os.Stat(lockPath); err == nil {
-		t.Fatal("stale secret lock was not cleaned up")
-	}
-
-	s2, err := NewStore(StoreOptions{FilePath: path, Encrypted: true})
-	if err != nil {
-		t.Fatalf("NewStore: %v", err)
-	}
-	token, ok, err := s2.Load(ProviderKey("demo"))
-	if err != nil || !ok || token.AccessToken != "after-crash" {
-		t.Fatalf("Load after stale-lock recovery = %+v ok=%v err=%v", token, ok, err)
-	}
-}

internal/oauth/publication_test.go

@@ -1,98 +0,0 @@
-package oauth
-
-import (
-	"bytes"
-	"errors"
-	"os"
-	"path/filepath"
-	"strings"
-	"testing"
-)
-
-func TestFileBlobWriteSyncsFileAndDirectory(t *testing.T) {
-	path := filepath.Join(t.TempDir(), "oauth-tokens.json")
-	blob := fileBlob{path: path}
-	origFile, origDir := syncOAuthFileFn, syncOAuthDirFn
-	t.Cleanup(func() {
-		syncOAuthFileFn, syncOAuthDirFn = origFile, origDir
-	})
-	fileSynced, dirSynced := false, false
-	syncOAuthFileFn = func(file *os.File) error {
-		fileSynced = true
-		return origFile(file)
-	}
-	syncOAuthDirFn = func(dir string) error {
-		dirSynced = true
-		return origDir(dir)
-	}
-
-	if err := blob.write([]byte("payload")); err != nil {
-		t.Fatalf("write: %v", err)
-	}
-	if !fileSynced {
-		t.Fatal("token blob was not synced before rename")
-	}
-	if !dirSynced {
-		t.Fatal("token store directory was not synced after rename")
-	}
-	data, ok, err := blob.read()
-	if err != nil || !ok || !bytes.Equal(data, []byte("payload")) {
-		t.Fatalf("read = %q ok=%v err=%v", data, ok, err)
-	}
-}
-
-// TestFileBlobWriteDirectorySyncFailureLeavesPublishedBlob pins the post-rename
-// path: the directory barrier runs after the atomic replace, so a barrier
-// failure reports an error but must not hide the already-published token.
-func TestFileBlobWriteDirectorySyncFailureLeavesPublishedBlob(t *testing.T) {
-	path := filepath.Join(t.TempDir(), "oauth-tokens.json")
-	blob := fileBlob{path: path}
-	if err := blob.write([]byte("old")); err != nil {
-		t.Fatalf("seed write: %v", err)
-	}
-
-	origDir := syncOAuthDirFn
-	t.Cleanup(func() { syncOAuthDirFn = origDir })
-	syncErr := errors.New("directory sync failed")
-	syncOAuthDirFn = func(string) error { return syncErr }
-
-	err := blob.write([]byte("new"))
-	if !errors.Is(err, syncErr) {
-		t.Fatalf("write error = %v, want it to wrap %v", err, syncErr)
-	}
-	data, ok, err := blob.read()
-	if err != nil || !ok || !bytes.Equal(data, []byte("new")) {
-		t.Fatalf("published blob was not retained: data=%q ok=%v err=%v", data, ok, err)
-	}
-}
-
-func TestFileBlobWriteSyncFailurePreservesPrevious(t *testing.T) {
-	path := filepath.Join(t.TempDir(), "oauth-tokens.json")
-	blob := fileBlob{path: path}
-	if err := blob.write([]byte("old")); err != nil {
-		t.Fatalf("seed write: %v", err)
-	}
-
-	origFile := syncOAuthFileFn
-	t.Cleanup(func() { syncOAuthFileFn = origFile })
-	syncErr := errors.New("sync failed")
-	syncOAuthFileFn = func(*os.File) error { return syncErr }
-
-	err := blob.write([]byte("new"))
-	if !errors.Is(err, syncErr) {
-		t.Fatalf("write error = %v, want it to wrap %v", err, syncErr)
-	}
-	data, ok, err := blob.read()
-	if err != nil || !ok || !bytes.Equal(data, []byte("old")) {
-		t.Fatalf("previous blob was not preserved: data=%q ok=%v err=%v", data, ok, err)
-	}
-	entries, err := os.ReadDir(PublicationDir(path))
-	if err != nil {
-		t.Fatalf("ReadDir publication dir: %v", err)
-	}
-	for _, entry := range entries {
-		if strings.HasPrefix(entry.Name(), "publish-") {
-			t.Fatalf("failed write left publication temp %s", entry.Name())
-		}
-	}
-}

internal/oauth/secret_access_other.go

@@ -0,0 +1,7 @@
+//go:build !windows
+
+package oauth
+
+func isTransientSecretAccessError(error) bool {
+	return false
+}

internal/oauth/secret_access_windows.go

@@ -0,0 +1,14 @@
+//go:build windows
+
+package oauth
+
+import (
+	"errors"
+
+	"golang.org/x/sys/windows"
+)
+
+func isTransientSecretAccessError(err error) bool {
+	return errors.Is(err, windows.ERROR_SHARING_VIOLATION) ||
+		errors.Is(err, windows.ERROR_LOCK_VIOLATION)
+}

internal/oauth/store.go

@@ -18,9 +18,7 @@ import (
 	"sync"
 	"time"
 
-	"github.com/Twigpine/zero/internal/fsutil"
 	"github.com/Twigpine/zero/internal/keyring"
-	"github.com/Twigpine/zero/internal/securefile"
 )
 
 const (
@@ -157,7 +155,7 @@ const (
 // is AES-256-GCM ciphertext at rest.
 type Store struct {
 	blob    blobStore
-	crypter *securefile.Crypter // nil => plaintext blob
+	crypter *aesGCMCrypter // nil => plaintext blob
 	now     func() time.Time
 	mu      sync.Mutex
 }
@@ -246,15 +244,8 @@ func NewStore(options StoreOptions) (*Store, error) {
 			return nil, err
 		}
 		// The file blob holds AES-256-GCM ciphertext; the per-user secret lives in
-		// a sibling ".secret" file. The secret publishes through the sandbox-denied
-		// ".secret.publish" directory so a same-user sandboxed process cannot race
-		// the plaintext key name.
-		secretPath := path + ".secret"
-		return &Store{
-			blob:    fileBlob{path: path},
-			crypter: securefile.NewCrypterWithPublicationDir(secretPath, PublicationDir(secretPath)),
-			now:     now,
-		}, nil
+		// a sibling ".secret" file (see encrypt.go).
+		return &Store{blob: fileBlob{path: path}, crypter: newAESGCMCrypter(path + ".secret"), now: now}, nil
 	case "keyring":
 		kr := options.Keyring
 		if kr == nil {
@@ -414,7 +405,7 @@ func (s *Store) readState() (storeFile, error) {
 	}
 	if s.crypter != nil {
 		// Encrypted backend: the blob is AES-256-GCM ciphertext, not JSON.
-		data, err = s.crypter.Open(data)
+		data, err = s.crypter.open(data)
 		if err != nil {
 			return storeFile{}, fmt.Errorf("oauth: decrypt token store at %s: %w", s.blob.location(), err)
 		}
@@ -446,9 +437,9 @@ func (s *Store) writeState(state storeFile) error {
 	// writes opaque ciphertext instead.
 	payload := append(data, '\n')
 	if s.crypter != nil {
-		payload, err = s.crypter.Seal(data)
+		payload, err = s.crypter.seal(data)
 		if err != nil {
-			return fmt.Errorf("oauth: encrypt token store: %w", err)
+			return err
 		}
 	}
 	return s.blob.write(payload)
@@ -520,31 +511,6 @@ func (b fileBlob) read() ([]byte, bool, error) {
 	return data, true, nil
 }
 
-// Sync seams allow tests to inject persistence failures at each barrier.
-var (
-	syncOAuthFileFn = (*os.File).Sync
-	syncOAuthDirFn  = syncOAuthDir
-)
-
-// syncOAuthDir fsyncs the directory that now holds the published token store.
-// Windows cannot sync directory handles, so the rename is best-effort durable
-// there. A directory the user can write but not read cannot be opened for
-// syncing either; that is also best effort, because the write it follows
-// already succeeded. Every other failure is reported.
-func syncOAuthDir(dir string) error {
-	if runtime.GOOS == "windows" {
-		return nil
-	}
-	d, err := os.Open(dir)
-	if err != nil {
-		if errors.Is(err, os.ErrPermission) {
-			return nil
-		}
-		return err
-	}
-	return errors.Join(d.Sync(), d.Close())
-}
-
 func (b fileBlob) write(data []byte) error {
 	if err := os.MkdirAll(filepath.Dir(b.path), 0o700); err != nil {
 		return err
@@ -558,20 +524,10 @@ func (b fileBlob) write(data []byte) error {
 		_ = temp.Close()
 		return err
 	}
-	if err := syncOAuthFileFn(temp); err != nil {
-		_ = temp.Close()
-		return fmt.Errorf("oauth: sync token store: %w", err)
-	}
 	if err := temp.Close(); err != nil {
-		return fmt.Errorf("oauth: write token store: %w", err)
+		return err
 	}
-	if err := fsutil.RenameWithRetry(tempPath, b.path, nil); err != nil {
-		return fmt.Errorf("oauth: publish token store: %w", err)
-	}
-	if err := syncOAuthDirFn(filepath.Dir(b.path)); err != nil {
-		return fmt.Errorf("oauth: sync token store directory: %w", err)
-	}
-	return nil
+	return os.Rename(tempPath, b.path)
 }
 
 // PublicationDirSuffix names the per-store directory a token store publishes new

internal/plugins/activate.go

@@ -374,7 +374,7 @@ func (tool skillTool) Run(_ context.Context, args map[string]any) tools.Result {
 	available := make([]string, 0, len(merged))
 	for _, skill := range merged {
 		if skill.Name == name {
-			return tools.Result{Status: tools.StatusOK, Output: skills.ToolOutput(skill)}
+			return tools.Result{Status: tools.StatusOK, Output: skill.Content}
 		}
 		available = append(available, skill.Name)
 	}

internal/plugins/activate_test.go

@@ -673,28 +673,3 @@ func TestExpandPluginRootPathRelative(t *testing.T) {
 		}
 	}
 }
-
-// The multi-root skill tool is the one the CLI registers, so it has to point at
-// a skill's bundled files the same way the core tool does (#584).
-func TestNewSkillToolPointsAtTheSkillsBundledFiles(t *testing.T) {
-	home := t.TempDir()
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	dir := t.TempDir()
-	writeTestSkill(t, dir, "pdf", "Run scripts/fill.py.")
-	script := filepath.Join(dir, "pdf", "scripts", "fill.py")
-	if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-
-	got := NewSkillTool(dir, nil).Run(context.Background(), map[string]any{"name": "pdf"})
-	if got.Status != tools.StatusOK {
-		t.Fatalf("Status = %s (output: %s)", got.Status, got.Output)
-	}
-	if !strings.Contains(got.Output, "- scripts/fill.py") || !strings.Contains(got.Output, filepath.Join(dir, "pdf")) {
-		t.Fatalf("output does not point at the bundled script:\n%s", got.Output)
-	}
-}

internal/securefile/securefile.go

@@ -15,7 +15,6 @@ import (
 	"io"
 	"os"
 	"path/filepath"
-	"runtime"
 	"sync/atomic"
 	"time"
 )
@@ -31,35 +30,13 @@ const (
 
 var (
 	openSecretLockFile = os.OpenFile
-	syncSecretFileFn   = (*os.File).Sync
-	syncSecretDirFn    = syncSecretDir
 	secureLockSeq      atomic.Uint64
 )
 
-// syncSecretDir fsyncs the directory that now holds the published secret.
-// Windows cannot sync directory handles, so the rename is best-effort durable
-// there. A directory the user can write but not read cannot be opened for
-// syncing either; that is also best effort, because the write it follows
-// already succeeded. Every other failure is reported.
-func syncSecretDir(dir string) error {
-	if runtime.GOOS == "windows" {
-		return nil
-	}
-	d, err := os.Open(dir)
-	if err != nil {
-		if errors.Is(err, os.ErrPermission) {
-			return nil
-		}
-		return err
-	}
-	return errors.Join(d.Sync(), d.Close())
-}
-
 // Crypter encrypts a blob at rest with AES-256-GCM under a per-user random secret
 // persisted (0600) at secretPath.
 type Crypter struct {
-	secretPath     string
-	publicationDir string
+	secretPath string
 }
 
 // NewCrypter returns a Crypter whose key lives at secretPath (created on first
@@ -68,20 +45,11 @@ func NewCrypter(secretPath string) *Crypter {
 	return &Crypter{secretPath: secretPath}
 }
 
-// NewCrypterWithPublicationDir is NewCrypter with secret temp files published
-// through publicationDir instead of beside the secret. Callers that protect a
-// sandboxed process from racing the plaintext key use this to put the temp file
-// in a directory their sandbox profile denies. The directory must be on the
-// same filesystem as secretPath; the final rename remains atomic.
-func NewCrypterWithPublicationDir(secretPath, publicationDir string) *Crypter {
-	return &Crypter{secretPath: secretPath, publicationDir: publicationDir}
-}
-
 // aead loads (or, when create is set, generates) the secret and returns the GCM
 // AEAD. Open passes create=false so a missing secret is a hard error rather than
 // silently minting a new key that could never decrypt the existing file.
 func (c *Crypter) aead(create bool) (cipher.AEAD, error) {
-	secret, err := loadOrCreateSecret(c.secretPath, c.publicationDir, create)
+	secret, err := loadOrCreateSecret(c.secretPath, create)
 	if err != nil {
 		return nil, err
 	}
@@ -127,7 +95,7 @@ func (c *Crypter) Open(blob []byte) ([]byte, error) {
 // loadOrCreateSecret reads the 32-byte secret at path. When create is set and the
 // file is absent, it generates a random secret and creates the file atomically
 // (0600). A wrong-sized existing secret fails closed (corruption).
-func loadOrCreateSecret(path, publicationDir string, create bool) ([]byte, error) {
+func loadOrCreateSecret(path string, create bool) ([]byte, error) {
 	if data, err := readSecretFileRetry(path); err == nil {
 		return data, nil
 	} else if !errors.Is(err, os.ErrNotExist) {
@@ -139,10 +107,10 @@ func loadOrCreateSecret(path, publicationDir string, create bool) ([]byte, error
 	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
 		return nil, err
 	}
-	return createSecretFile(path, publicationDir)
+	return createSecretFile(path)
 }
 
-func createSecretFile(path, publicationDir string) ([]byte, error) {
+func createSecretFile(path string) ([]byte, error) {
 	lockPath := path + ".lock"
 	token := fmt.Sprintf("%d-%d-%d", os.Getpid(), time.Now().UnixNano(), secureLockSeq.Add(1))
 	var lastErr error
@@ -168,13 +136,13 @@ func createSecretFile(path, publicationDir string) ([]byte, error) {
 			} else if !errors.Is(rerr, os.ErrNotExist) {
 				return nil, rerr
 			}
-			return writeNewSecretFile(path, publicationDir)
+			return writeNewSecretFile(path)
 		}
 		// On Windows a concurrent holder's os.Remove leaves the lock file in a
 		// "delete pending" state, so an O_EXCL create races it with
 		// ERROR_ACCESS_DENIED (os.ErrPermission) rather than ErrExist. Treat that
-		// as contention and retry, otherwise concurrent secret creation
-		// spuriously fails on Windows.
+		// as contention and retry too -- mirroring oauth's createSecretFile --
+		// otherwise concurrent secret creation spuriously fails on Windows.
 		if !errors.Is(err, os.ErrExist) && !errors.Is(err, os.ErrPermission) {
 			return nil, fmt.Errorf("securefile: create secret lock: %w", err)
 		}
@@ -205,63 +173,20 @@ func reclaimStaleLock(lockPath, token string, staleAfter time.Duration) bool {
 		return false
 	}
 	if info, err := os.Stat(reclaimed); err == nil && time.Since(info.ModTime()) <= staleAfter {
-		// The lock was replaced between our stat and rename, so put the fresh
-		// lock back without clobbering a third holder that acquired lockPath in
-		// the meantime.
-		_ = restoreReclaimedLock(reclaimed, lockPath)
+		_ = os.Rename(reclaimed, lockPath)
 		return false
 	}
 	_ = os.Remove(reclaimed)
 	return true
 }
 
-// restoreReclaimedLock restores a quarantined lock only if lockPath is still
-// free. The destination is created exclusively; a newer holder is never
-// replaced. The quarantined copy is discarded either way.
-func restoreReclaimedLock(reclaimed, lockPath string) error {
-	data, err := os.ReadFile(reclaimed)
-	if err != nil {
-		_ = os.Remove(reclaimed)
-		return err
-	}
-	lock, err := openSecretLockFile(lockPath, os.O_CREATE|os.O_EXCL|os.O_WRONLY, 0o600)
-	if err != nil {
-		_ = os.Remove(reclaimed)
-		return err
-	}
-	if _, err := lock.Write(data); err != nil {
-		_ = lock.Close()
-		_ = os.Remove(lockPath)
-		_ = os.Remove(reclaimed)
-		return err
-	}
-	if err := lock.Close(); err != nil {
-		_ = os.Remove(lockPath)
-		_ = os.Remove(reclaimed)
-		return err
-	}
-	_ = os.Remove(reclaimed)
-	return nil
-}
-
-func writeNewSecretFile(path, publicationDir string) ([]byte, error) {
+func writeNewSecretFile(path string) ([]byte, error) {
 	secret := make([]byte, secretBytes)
 	if _, err := io.ReadFull(rand.Reader, secret); err != nil {
 		return nil, fmt.Errorf("securefile: generate secret: %w", err)
 	}
-	dir := publicationDir
-	prefix := filepath.Base(path) + ".*.tmp"
-	if dir == "" {
-		dir = filepath.Dir(path)
-	} else {
-		// Keep the prefix oauth's store reset uses to clean crash residue from a
-		// publication directory.
-		prefix = "publish-*"
-	}
-	if err := os.MkdirAll(dir, 0o700); err != nil {
-		return nil, fmt.Errorf("securefile: create secret publication dir: %w", err)
-	}
-	tmp, err := os.CreateTemp(dir, prefix)
+	dir := filepath.Dir(path)
+	tmp, err := os.CreateTemp(dir, filepath.Base(path)+".*.tmp")
 	if err != nil {
 		return nil, fmt.Errorf("securefile: create secret temp file: %w", err)
 	}
@@ -275,7 +200,7 @@ func writeNewSecretFile(path, publicationDir string) ([]byte, error) {
 		_ = tmp.Close()
 		return nil, fmt.Errorf("securefile: write secret: %w", werr)
 	}
-	if serr := syncSecretFileFn(tmp); serr != nil {
+	if serr := tmp.Sync(); serr != nil {
 		_ = tmp.Close()
 		return nil, fmt.Errorf("securefile: sync secret: %w", serr)
 	}
@@ -285,12 +210,6 @@ func writeNewSecretFile(path, publicationDir string) ([]byte, error) {
 	if err := os.Rename(tmpPath, path); err != nil {
 		return nil, fmt.Errorf("securefile: publish secret: %w", err)
 	}
-	// The key is now visible at its final name; sync the destination directory
-	// so the rename itself survives a crash. The source publication directory
-	// only held an unlinked temp entry and does not need a barrier.
-	if err := syncSecretDirFn(filepath.Dir(path)); err != nil {
-		return nil, fmt.Errorf("securefile: sync secret directory: %w", err)
-	}
 	return secret, nil
 }
 

internal/securefile/securefile_test.go

@@ -1,12 +1,9 @@
 package securefile
 
 import (
-	"bytes"
-	"errors"
 	"os"
 	"path/filepath"
 	"runtime"
-	"sync"
 	"testing"
 	"time"
 )
@@ -26,7 +23,7 @@ func TestCreateSecretFileRetriesOnPermissionContention(t *testing.T) {
 		openSecretLockFile = origOpen
 	})
 
-	secret, err := createSecretFile(secretPath, "")
+	secret, err := createSecretFile(secretPath)
 	if err != nil {
 		t.Fatalf("createSecretFile should retry on permission contention: %v", err)
 	}
@@ -128,7 +125,7 @@ func TestCreateSecretFileReclaimsStaleLock(t *testing.T) {
 	}
 
 	// Attempt to create the secret file; it should detect the stale lock, reclaim it, and succeed
-	secret, err := createSecretFile(secretPath, "")
+	secret, err := createSecretFile(secretPath)
 	if err != nil {
 		t.Fatalf("createSecretFile failed to reclaim stale lock: %v", err)
 	}
@@ -141,198 +138,3 @@ func TestCreateSecretFileReclaimsStaleLock(t *testing.T) {
 		t.Fatal("expected lock file to be cleaned up after successful acquisition")
 	}
 }
-
-func TestSecretCreationConcurrentConverges(t *testing.T) {
-	secretPath := filepath.Join(t.TempDir(), "k.secret")
-	const n = 16
-	blobs := make([][]byte, n)
-	errs := make([]error, n)
-	var wg sync.WaitGroup
-	for i := 0; i < n; i++ {
-		wg.Add(1)
-		go func(i int) {
-			defer wg.Done()
-			blobs[i], errs[i] = NewCrypter(secretPath).Seal([]byte("payload"))
-		}(i)
-	}
-	wg.Wait()
-
-	c := NewCrypter(secretPath)
-	for i := 0; i < n; i++ {
-		if errs[i] != nil {
-			t.Fatalf("goroutine %d: %v", i, errs[i])
-		}
-		got, err := c.Open(blobs[i])
-		if err != nil {
-			t.Fatalf("goroutine %d blob does not decrypt under the converged secret: %v", i, err)
-		}
-		if !bytes.Equal(got, []byte("payload")) {
-			t.Fatalf("goroutine %d plaintext = %q", i, got)
-		}
-	}
-}
-
-func TestWrongSizedSecretFailsClosed(t *testing.T) {
-	secretPath := filepath.Join(t.TempDir(), "k.secret")
-	if err := os.WriteFile(secretPath, []byte("short"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-	if _, err := NewCrypter(secretPath).Seal([]byte("payload")); err == nil {
-		t.Fatal("wrong-sized secret must error, not mint a new key")
-	}
-}
-
-func TestNewCrypterWithPublicationDirPublishesThroughIt(t *testing.T) {
-	dir := t.TempDir()
-	secretPath := filepath.Join(dir, "tokens.json.secret")
-	pubDir := secretPath + ".publish"
-	c := NewCrypterWithPublicationDir(secretPath, pubDir)
-	if _, err := c.Seal([]byte("payload")); err != nil {
-		t.Fatalf("Seal: %v", err)
-	}
-	if info, err := os.Stat(secretPath); err != nil || info.Size() != secretBytes {
-		t.Fatalf("secret = %v err=%v", info, err)
-	}
-	entries, err := os.ReadDir(pubDir)
-	if err != nil {
-		t.Fatalf("publication dir: %v", err)
-	}
-	if len(entries) != 0 {
-		t.Fatalf("publication dir left temp files: %v", entries)
-	}
-	if entries, err := os.ReadDir(dir); err != nil {
-		t.Fatalf("parent dir: %v", err)
-	} else {
-		for _, entry := range entries {
-			if entry.Name() != "tokens.json.secret" && entry.Name() != filepath.Base(pubDir) {
-				t.Fatalf("unexpected parent-dir entry %q; secret must publish through the protected dir", entry.Name())
-			}
-		}
-	}
-}
-
-func TestSecretCreationSyncsFileAndDirectory(t *testing.T) {
-	secretPath := filepath.Join(t.TempDir(), "k.secret")
-	origFile, origDir := syncSecretFileFn, syncSecretDirFn
-	t.Cleanup(func() {
-		syncSecretFileFn, syncSecretDirFn = origFile, origDir
-	})
-	fileSynced, dirSynced := false, false
-	syncSecretFileFn = func(file *os.File) error {
-		fileSynced = true
-		return origFile(file)
-	}
-	syncSecretDirFn = func(dir string) error {
-		dirSynced = true
-		return origDir(dir)
-	}
-	if _, err := createSecretFile(secretPath, ""); err != nil {
-		t.Fatalf("createSecretFile: %v", err)
-	}
-	if !fileSynced {
-		t.Fatal("secret temp file was not synced before rename")
-	}
-	if !dirSynced {
-		t.Fatal("secret destination directory was not synced after rename")
-	}
-}
-
-// TestSecretDirectorySyncFailureLeavesPublishedSecret pins the post-rename
-// path: a destination-directory barrier failure reports an error but the
-// renamed secret is already there and a later caller can load it.
-func TestSecretDirectorySyncFailureLeavesPublishedSecret(t *testing.T) {
-	secretPath := filepath.Join(t.TempDir(), "k.secret")
-	origDir := syncSecretDirFn
-	t.Cleanup(func() { syncSecretDirFn = origDir })
-	syncErr := errors.New("directory sync failed")
-	syncSecretDirFn = func(string) error { return syncErr }
-
-	if _, err := createSecretFile(secretPath, ""); !errors.Is(err, syncErr) {
-		t.Fatalf("createSecretFile error = %v, want %v", err, syncErr)
-	}
-	published, err := os.ReadFile(secretPath)
-	if err != nil {
-		t.Fatalf("read published secret: %v", err)
-	}
-	if len(published) != secretBytes {
-		t.Fatalf("published secret size = %d, want %d", len(published), secretBytes)
-	}
-	loaded, err := createSecretFile(secretPath, "")
-	if err != nil {
-		t.Fatalf("later caller could not load the published secret: %v", err)
-	}
-	if !bytes.Equal(loaded, published) {
-		t.Fatal("later caller did not reuse the published secret")
-	}
-}
-
-// TestRestoreReclaimedLockDoesNotReplaceNewerLock covers the reclaim race: a
-// lock that looked stale can be replaced before the reclaimer renames it. If a
-// third holder then creates lockPath, restoring the quarantined lock must not
-// replace that newer lock.
-func TestRestoreReclaimedLockDoesNotReplaceNewerLock(t *testing.T) {
-	dir := t.TempDir()
-	lockPath := filepath.Join(dir, "k.secret.lock")
-	reclaimed := lockPath + ".stale.crashed"
-	if err := os.WriteFile(reclaimed, []byte("quarantined-token"), 0o600); err != nil {
-		t.Fatalf("write quarantined lock: %v", err)
-	}
-	if err := os.WriteFile(lockPath, []byte("new-holder-token"), 0o600); err != nil {
-		t.Fatalf("write newer lock: %v", err)
-	}
-
-	if err := restoreReclaimedLock(reclaimed, lockPath); !errors.Is(err, os.ErrExist) {
-		t.Fatalf("restoreReclaimedLock error = %v, want %v", err, os.ErrExist)
-	}
-	data, err := os.ReadFile(lockPath)
-	if err != nil {
-		t.Fatalf("read newer lock: %v", err)
-	}
-	if string(data) != "new-holder-token" {
-		t.Fatalf("newer lock was replaced: %q", data)
-	}
-	if _, err := os.Stat(reclaimed); !errors.Is(err, os.ErrNotExist) {
-		t.Fatalf("quarantined lock was not discarded: %v", err)
-	}
-}
-
-func TestRestoreReclaimedLockRestoresWhenDestinationIsFree(t *testing.T) {
-	dir := t.TempDir()
-	lockPath := filepath.Join(dir, "k.secret.lock")
-	reclaimed := lockPath + ".stale.crashed"
-	if err := os.WriteFile(reclaimed, []byte("fresh-token"), 0o600); err != nil {
-		t.Fatalf("write quarantined lock: %v", err)
-	}
-
-	if err := restoreReclaimedLock(reclaimed, lockPath); err != nil {
-		t.Fatalf("restoreReclaimedLock: %v", err)
-	}
-	data, err := os.ReadFile(lockPath)
-	if err != nil {
-		t.Fatalf("read restored lock: %v", err)
-	}
-	if string(data) != "fresh-token" {
-		t.Fatalf("restored lock = %q", data)
-	}
-	if _, err := os.Stat(reclaimed); !errors.Is(err, os.ErrNotExist) {
-		t.Fatalf("quarantined lock was not discarded: %v", err)
-	}
-}
-
-func TestSecretSyncFailureLeavesNoSecretAndRetrySucceeds(t *testing.T) {
-	secretPath := filepath.Join(t.TempDir(), "k.secret")
-	origFile := syncSecretFileFn
-	t.Cleanup(func() { syncSecretFileFn = origFile })
-	syncErr := errors.New("sync failed")
-	syncSecretFileFn = func(*os.File) error { return syncErr }
-	if _, err := createSecretFile(secretPath, ""); !errors.Is(err, syncErr) {
-		t.Fatalf("createSecretFile error = %v, want %v", err, syncErr)
-	}
-	if _, err := os.Stat(secretPath); !errors.Is(err, os.ErrNotExist) {
-		t.Fatalf("failed publish left a secret: %v", err)
-	}
-	syncSecretFileFn = origFile
-	if _, err := createSecretFile(secretPath, ""); err != nil {
-		t.Fatalf("retry after sync failure: %v", err)
-	}
-}

internal/skills/bundle.go

@@ -1,258 +0,0 @@
-package skills
-
-import (
-	"crypto/sha256"
-	"encoding/hex"
-	"errors"
-	"fmt"
-	"io"
-	"io/fs"
-	"os"
-	"path/filepath"
-	"sort"
-	"strings"
-)
-
-// A skill is a directory, not just its SKILL.md: the instructions often point at
-// scripts, references or assets that sit beside them (the agentskills.io
-// layout). Install copies that directory, and the skill tool says where it is.
-
-// Install refuses a skill tree larger than this. A skill is a handful of
-// scripts and documents; a source that is really a whole repository or a pile
-// of binaries should fail loudly instead of filling the skills directory.
-var (
-	maxSkillTreeFiles = 1000
-	maxSkillTreeBytes = int64(32 << 20)
-)
-
-// bundledFileListLimit caps how many bundled files the skill tool lists, so a
-// large skill does not crowd the instructions out of the tool output.
-const bundledFileListLimit = 50
-
-// skipSkillEntry reports whether a skill tree entry is left out of an install,
-// a tree hash and a listing alike. Hidden entries cover VCS metadata (.git,
-// which a cloned source always has) and local clutter such as .DS_Store or
-// .env; they are not part of what a skill ships.
-func skipSkillEntry(name string) bool {
-	return strings.HasPrefix(name, ".")
-}
-
-// copySkillTree copies the skill directory src into dst, which must exist.
-// Regular files are copied with normalized permissions (0o755 when the source
-// is executable, 0o644 otherwise), so a script stays runnable without carrying
-// over group- or world-writable bits. Symlinks are not followed or copied:
-// their relative paths come back in skipped, so the caller can say so. Other
-// non-regular entries (devices, sockets, pipes) are left out. bundled counts
-// the files copied besides SKILL.md.
-func copySkillTree(src, dst string) (bundled int, skipped []string, err error) {
-	files := 0
-	var total int64
-	err = filepath.WalkDir(src, func(path string, entry fs.DirEntry, walkErr error) error {
-		if walkErr != nil {
-			return walkErr
-		}
-		if path == src {
-			return nil
-		}
-		rel, err := filepath.Rel(src, path)
-		if err != nil {
-			return err
-		}
-		if skipSkillEntry(entry.Name()) {
-			if entry.IsDir() {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if entry.Type()&fs.ModeSymlink != 0 {
-			skipped = append(skipped, filepath.ToSlash(rel))
-			return nil
-		}
-		target := filepath.Join(dst, rel)
-		if entry.IsDir() {
-			return os.MkdirAll(target, 0o755)
-		}
-		if !entry.Type().IsRegular() {
-			return nil
-		}
-		info, err := entry.Info()
-		if err != nil {
-			return err
-		}
-		files++
-		total += info.Size()
-		if files > maxSkillTreeFiles {
-			return fmt.Errorf("skill has more than %d files; a skill directory should hold only what the skill uses", maxSkillTreeFiles)
-		}
-		if total > maxSkillTreeBytes {
-			return fmt.Errorf("skill is larger than %d MiB; a skill directory should hold only what the skill uses", maxSkillTreeBytes>>20)
-		}
-		if filepath.ToSlash(rel) != skillFileName {
-			bundled++
-		}
-		mode := os.FileMode(0o644)
-		if info.Mode().Perm()&0o111 != 0 {
-			mode = 0o755
-		}
-		return copySkillFile(path, target, mode)
-	})
-	if err != nil {
-		return 0, nil, err
-	}
-	return bundled, skipped, nil
-}
-
-func copySkillFile(src, dst string, mode os.FileMode) error {
-	in, err := os.Open(src)
-	if err != nil {
-		return err
-	}
-	defer in.Close()
-	out, err := os.OpenFile(dst, os.O_WRONLY|os.O_CREATE|os.O_EXCL, mode)
-	if err != nil {
-		return err
-	}
-	if _, err := io.Copy(out, in); err != nil {
-		_ = out.Close()
-		return err
-	}
-	if err := out.Close(); err != nil {
-		return err
-	}
-	// The create mode is filtered by the umask; set it outright so an
-	// executable script is still executable wherever the install ran.
-	return os.Chmod(dst, mode)
-}
-
-// skillTreeHash hashes every file a skill ships, with the same entries
-// copySkillTree copies: its relative path, whether it is executable, and its
-// content. Sorted by path, so the hash depends only on the tree.
-func skillTreeHash(dir string) (string, error) {
-	type entry struct {
-		rel  string
-		exec bool
-		sum  string
-	}
-	var entries []entry
-	err := filepath.WalkDir(dir, func(path string, item fs.DirEntry, walkErr error) error {
-		if walkErr != nil {
-			return walkErr
-		}
-		if path == dir {
-			return nil
-		}
-		if skipSkillEntry(item.Name()) {
-			if item.IsDir() {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if !item.Type().IsRegular() {
-			return nil
-		}
-		info, err := item.Info()
-		if err != nil {
-			return err
-		}
-		data, err := os.ReadFile(path)
-		if err != nil {
-			return err
-		}
-		rel, err := filepath.Rel(dir, path)
-		if err != nil {
-			return err
-		}
-		sum := sha256.Sum256(data)
-		entries = append(entries, entry{rel: filepath.ToSlash(rel), exec: info.Mode().Perm()&0o111 != 0, sum: hex.EncodeToString(sum[:])})
-		return nil
-	})
-	if err != nil {
-		return "", err
-	}
-	sort.Slice(entries, func(i, j int) bool { return entries[i].rel < entries[j].rel })
-	hash := sha256.New()
-	for _, item := range entries {
-		flag := "-"
-		if item.exec {
-			flag = "x"
-		}
-		// Length-prefixed, so no path can be read as the start of another entry.
-		fmt.Fprintf(hash, "%d:%s|%s|%s;", len(item.rel), item.rel, flag, item.sum)
-	}
-	return "sha256:" + hex.EncodeToString(hash.Sum(nil)), nil
-}
-
-// BundledFiles lists the files a skill ships besides its SKILL.md, as
-// slash-separated paths relative to the skill's directory, sorted. Hidden
-// entries and symlinks are left out: the listing tells the model what the
-// skill brings, and is not a way to reach anything outside it. total is the
-// number of files found; at most limit come back.
-func BundledFiles(skill Skill, limit int) (files []string, total int, err error) {
-	dir := filepath.Dir(skill.Path)
-	if strings.TrimSpace(skill.Path) == "" {
-		return nil, 0, errors.New("skill has no path")
-	}
-	err = filepath.WalkDir(dir, func(path string, entry fs.DirEntry, walkErr error) error {
-		if walkErr != nil {
-			return walkErr
-		}
-		if path == dir {
-			return nil
-		}
-		if skipSkillEntry(entry.Name()) {
-			if entry.IsDir() {
-				return filepath.SkipDir
-			}
-			return nil
-		}
-		if !entry.Type().IsRegular() {
-			return nil
-		}
-		rel, err := filepath.Rel(dir, path)
-		if err != nil {
-			return err
-		}
-		rel = filepath.ToSlash(rel)
-		if rel == skillFileName {
-			return nil
-		}
-		total++
-		if total > maxSkillTreeFiles {
-			// A directory this large is not something the listing should walk
-			// to the end of; the count stops being exact past the cap.
-			return filepath.SkipAll
-		}
-		files = append(files, rel)
-		return nil
-	})
-	if err != nil {
-		return nil, 0, err
-	}
-	sort.Strings(files)
-	if limit >= 0 && len(files) > limit {
-		files = files[:limit]
-	}
-	return files, total, nil
-}
-
-// ToolOutput is what the skill tool returns for a loaded skill: its SKILL.md
-// body and, when the skill ships other files, the directory they are in and
-// what they are, so instructions that say "run scripts/extract.py" can be
-// followed. A skill that is only a SKILL.md gets its body back unchanged.
-func ToolOutput(skill Skill) string {
-	files, total, err := BundledFiles(skill, bundledFileListLimit)
-	if err != nil || total == 0 {
-		return skill.Content
-	}
-	var builder strings.Builder
-	builder.WriteString(strings.TrimRight(skill.Content, "\n"))
-	builder.WriteString("\n\n---\n")
-	fmt.Fprintf(&builder, "This skill's files are in %s. Paths in the instructions above are relative to that directory.\n", filepath.Dir(skill.Path))
-	for _, file := range files {
-		builder.WriteString("- " + file + "\n")
-	}
-	if more := total - len(files); more > 0 {
-		fmt.Fprintf(&builder, "- (and %d more)\n", more)
-	}
-	return builder.String()
-}

internal/skills/bundle_test.go

@@ -1,351 +0,0 @@
-package skills
-
-import (
-	"context"
-	"encoding/json"
-	"fmt"
-	"os"
-	"path/filepath"
-	"runtime"
-	"strings"
-	"testing"
-)
-
-const bundledSkillManifest = "---\nname: pdf\ndescription: fill PDF forms\n---\nRun scripts/fill.py with the form path.\n"
-
-// writeBundledSourceSkill lays down a skill in the agentskills.io shape: a
-// SKILL.md plus the scripts, references and assets it points at, and the hidden
-// clutter a cloned source carries.
-func writeBundledSourceSkill(t *testing.T, dir string) string {
-	t.Helper()
-	writeSourceSkill(t, dir, bundledSkillManifest)
-	files := map[string]string{
-		"scripts/fill.py":    "print('fill')\n",
-		"references/form.md": "# Form fields\n",
-		"assets/logo.svg":    "<svg/>\n",
-		"LICENSE.txt":        "MIT\n",
-		".git/config":        "[core]\n",
-		".env":               "TOKEN=local-only\n",
-		"scripts/.cache/tmp": "scratch\n",
-	}
-	for rel, content := range files {
-		path := filepath.Join(dir, filepath.FromSlash(rel))
-		if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-			t.Fatal(err)
-		}
-		mode := os.FileMode(0o644)
-		if strings.HasPrefix(rel, "scripts/") {
-			mode = 0o755
-		}
-		if err := os.WriteFile(path, []byte(content), mode); err != nil {
-			t.Fatal(err)
-		}
-	}
-	return dir
-}
-
-func TestInstallCopiesTheSkillDirectory(t *testing.T) {
-	destDir := t.TempDir()
-	src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-
-	result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-	if err != nil {
-		t.Fatalf("Install: %v", err)
-	}
-	installed := filepath.Join(destDir, "pdf")
-	for _, rel := range []string{"SKILL.md", "scripts/fill.py", "references/form.md", "assets/logo.svg", "LICENSE.txt"} {
-		if _, err := os.Stat(filepath.Join(installed, filepath.FromSlash(rel))); err != nil {
-			t.Errorf("%s was not installed: %v", rel, err)
-		}
-	}
-	for _, rel := range []string{".git", ".env", "scripts/.cache"} {
-		if _, err := os.Stat(filepath.Join(installed, filepath.FromSlash(rel))); err == nil {
-			t.Errorf("hidden %s was installed; it is not part of the skill", rel)
-		}
-	}
-	if !strings.HasPrefix(result.TreeHash, "sha256:") {
-		t.Fatalf("TreeHash = %q, want a sha256 tree hash", result.TreeHash)
-	}
-	lock, err := ReadLock(destDir)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if lock["pdf"].TreeHash != result.TreeHash || lock["pdf"].Hash != result.Hash {
-		t.Fatalf("lock entry = %+v, want the install's hashes %q and %q", lock["pdf"], result.Hash, result.TreeHash)
-	}
-	if runtime.GOOS == "windows" {
-		return
-	}
-	script, err := os.Stat(filepath.Join(installed, "scripts", "fill.py"))
-	if err != nil {
-		t.Fatal(err)
-	}
-	if script.Mode().Perm() != 0o755 {
-		t.Errorf("installed script mode = %v, want 0755 so the skill can still run it", script.Mode().Perm())
-	}
-	reference, err := os.Stat(filepath.Join(installed, "references", "form.md"))
-	if err != nil {
-		t.Fatal(err)
-	}
-	if reference.Mode().Perm() != 0o644 {
-		t.Errorf("installed reference mode = %v, want 0644", reference.Mode().Perm())
-	}
-}
-
-func TestInstallSkipsSymlinksAndSaysSo(t *testing.T) {
-	destDir := t.TempDir()
-	src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-	outside := filepath.Join(t.TempDir(), "secret.txt")
-	if err := os.WriteFile(outside, []byte("not part of the skill\n"), 0o600); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Symlink(outside, filepath.Join(src, "references", "linked.md")); err != nil {
-		t.Skipf("symlinks unavailable: %v", err)
-	}
-
-	result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-	if err != nil {
-		t.Fatalf("Install: %v", err)
-	}
-	if len(result.SkippedLinks) != 1 || result.SkippedLinks[0] != "references/linked.md" {
-		t.Fatalf("SkippedLinks = %v, want [references/linked.md]", result.SkippedLinks)
-	}
-	if _, err := os.Lstat(filepath.Join(destDir, "pdf", "references", "linked.md")); err == nil {
-		t.Fatal("a symlink from the source was installed")
-	}
-}
-
-func TestInstallRefusesAnOversizedSkill(t *testing.T) {
-	for _, tc := range []struct {
-		name  string
-		files int
-		bytes int64
-		want  string
-	}{
-		{name: "too many files", files: 3, bytes: maxSkillTreeBytes, want: "more than 3 files"},
-		{name: "too many bytes", files: maxSkillTreeFiles, bytes: 1 << 20, want: "larger than 1 MiB"},
-	} {
-		t.Run(tc.name, func(t *testing.T) {
-			files, bytes := maxSkillTreeFiles, maxSkillTreeBytes
-			t.Cleanup(func() { maxSkillTreeFiles, maxSkillTreeBytes = files, bytes })
-			maxSkillTreeFiles, maxSkillTreeBytes = tc.files, tc.bytes
-
-			destDir := t.TempDir()
-			src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-			big := make([]byte, 2<<20)
-			if err := os.WriteFile(filepath.Join(src, "assets", "big.bin"), big, 0o644); err != nil {
-				t.Fatal(err)
-			}
-			_, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-			if err == nil || !strings.Contains(err.Error(), tc.want) {
-				t.Fatalf("Install error = %v, want %q", err, tc.want)
-			}
-			if _, statErr := os.Stat(filepath.Join(destDir, "pdf")); statErr == nil {
-				t.Fatal("a refused install left a skill directory behind")
-			}
-		})
-	}
-}
-
-func TestReinstallWithOnlyABundledFileChangedIsAnUpdate(t *testing.T) {
-	destDir := t.TempDir()
-	src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-	first, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-	if err != nil {
-		t.Fatalf("first Install: %v", err)
-	}
-	if err := os.WriteFile(filepath.Join(src, "scripts", "fill.py"), []byte("print('fill v2')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-
-	second, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-	if err != nil {
-		t.Fatalf("second Install: %v", err)
-	}
-	if !second.Updated {
-		t.Fatal("a reinstall that changed a bundled script was not reported as an update")
-	}
-	if second.Hash != first.Hash || second.TreeHash == first.TreeHash {
-		t.Fatalf("hashes = %q/%q after %q/%q, want the same SKILL.md hash and a new tree hash", second.Hash, second.TreeHash, first.Hash, first.TreeHash)
-	}
-	if second.PreviousHash != first.Hash || second.PreviousTreeHash != first.TreeHash {
-		t.Fatalf("previous hashes = %q/%q, want the first install's %q/%q", second.PreviousHash, second.PreviousTreeHash, first.Hash, first.TreeHash)
-	}
-	got, err := os.ReadFile(filepath.Join(destDir, "pdf", "scripts", "fill.py"))
-	if err != nil || string(got) != "print('fill v2')\n" {
-		t.Fatalf("installed script = %q, %v; want the new version", got, err)
-	}
-}
-
-func TestInfoReportsDriftInABundledFile(t *testing.T) {
-	destDir := t.TempDir()
-	src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-	if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
-		t.Fatalf("Install: %v", err)
-	}
-	if info, ok := Info(destDir, "pdf"); !ok || info.HashDrift {
-		t.Fatalf("a fresh install reports drift: %+v, %v", info, ok)
-	}
-	script := filepath.Join(destDir, "pdf", "scripts", "fill.py")
-	if err := os.WriteFile(script, []byte("print('edited by hand')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if info, _ := Info(destDir, "pdf"); !info.HashDrift {
-		t.Fatal("an edited bundled script is not reported as drift")
-	}
-}
-
-// A lock entry written before installs carried their bundled files has no tree
-// hash. Checking its tree would report every such skill as drifted, so only
-// its SKILL.md is checked, as before.
-func TestLockEntryWithoutATreeHashChecksOnlySKILLmd(t *testing.T) {
-	destDir := t.TempDir()
-	src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-	if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
-		t.Fatalf("Install: %v", err)
-	}
-	dropTreeHash(t, destDir, "pdf")
-
-	if err := os.WriteFile(filepath.Join(destDir, "pdf", "scripts", "fill.py"), []byte("print('edited')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if info, _ := Info(destDir, "pdf"); info.HashDrift {
-		t.Fatal("an entry without a tree hash reported drift for a bundled file")
-	}
-	if err := os.WriteFile(filepath.Join(destDir, "pdf", skillFileName), []byte(bundledSkillManifest+"edited\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	if info, _ := Info(destDir, "pdf"); !info.HashDrift {
-		t.Fatal("an edited SKILL.md under an old lock entry is not reported as drift")
-	}
-}
-
-// Reinstalling over an entry from before tree hashes compares with what that
-// install left, SKILL.md alone: an unchanged skill that is only a SKILL.md is
-// not an update, and one whose bundled files this install adds is.
-func TestReinstallOverAnEntryWithoutATreeHash(t *testing.T) {
-	t.Run("SKILL.md only", func(t *testing.T) {
-		destDir := t.TempDir()
-		src := writeSourceSkill(t, filepath.Join(t.TempDir(), "pdf"), bundledSkillManifest)
-		if _, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir}); err != nil {
-			t.Fatalf("first Install: %v", err)
-		}
-		dropTreeHash(t, destDir, "pdf")
-
-		result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-		if err != nil {
-			t.Fatalf("second Install: %v", err)
-		}
-		if result.Updated {
-			t.Fatalf("reinstalling an unchanged SKILL.md-only skill over an old entry was reported as an update: %+v", result)
-		}
-		lock, err := ReadLock(destDir)
-		if err != nil {
-			t.Fatal(err)
-		}
-		if lock["pdf"].TreeHash != result.TreeHash {
-			t.Fatalf("lock tree hash = %q after the reinstall, want %q", lock["pdf"].TreeHash, result.TreeHash)
-		}
-	})
-	t.Run("with bundled files", func(t *testing.T) {
-		destDir := t.TempDir()
-		src := writeBundledSourceSkill(t, filepath.Join(t.TempDir(), "pdf"))
-		first, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-		if err != nil {
-			t.Fatalf("first Install: %v", err)
-		}
-		dropTreeHash(t, destDir, "pdf")
-
-		result, err := Install(context.Background(), InstallOptions{Source: src, Dir: destDir})
-		if err != nil {
-			t.Fatalf("second Install: %v", err)
-		}
-		if !result.Updated || result.Hash != first.Hash || result.PreviousHash != first.Hash || result.PreviousTreeHash != "" {
-			t.Fatalf("result = %+v, want an update with SKILL.md hash %q unchanged and no previous tree hash", result, first.Hash)
-		}
-	})
-}
-
-// dropTreeHash rewrites the lock entry for name the way an install from before
-// tree hashes left it: a source and a SKILL.md hash, nothing for the tree.
-func dropTreeHash(t *testing.T, destDir, name string) {
-	t.Helper()
-	lock, err := ReadLock(destDir)
-	if err != nil {
-		t.Fatal(err)
-	}
-	entry := lock[name]
-	entry.TreeHash = ""
-	lock[name] = entry
-	data, err := json.Marshal(lock)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(destDir, LockFileName), data, 0o644); err != nil {
-		t.Fatal(err)
-	}
-}
-
-func TestToolOutputSaysWhereTheBundledFilesAre(t *testing.T) {
-	root := t.TempDir()
-	writeBundledSourceSkill(t, filepath.Join(root, "pdf"))
-	skill, ok := Get(root, "pdf")
-	if !ok {
-		t.Fatal("skill not loaded")
-	}
-
-	output := ToolOutput(skill)
-	if !strings.HasPrefix(output, strings.TrimRight(skill.Content, "\n")) {
-		t.Fatalf("the instructions do not come first:\n%s", output)
-	}
-	if !strings.Contains(output, filepath.Dir(skill.Path)) {
-		t.Fatalf("the output does not say where the skill's files are:\n%s", output)
-	}
-	for _, want := range []string{"- LICENSE.txt", "- assets/logo.svg", "- references/form.md", "- scripts/fill.py"} {
-		if !strings.Contains(output, want+"\n") {
-			t.Errorf("output does not list %q:\n%s", want, output)
-		}
-	}
-	for _, hidden := range []string{".env", ".git", ".cache", "SKILL.md\n"} {
-		if strings.Contains(output, "- "+hidden) {
-			t.Errorf("output lists %q:\n%s", hidden, output)
-		}
-	}
-}
-
-func TestToolOutputCapsALongListing(t *testing.T) {
-	root := t.TempDir()
-	dir := writeSourceSkill(t, filepath.Join(root, "many"), "---\nname: many\n---\nbody\n")
-	for i := 0; i < bundledFileListLimit+7; i++ {
-		path := filepath.Join(dir, "references", fmt.Sprintf("part-%03d.md", i))
-		if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-			t.Fatal(err)
-		}
-		if err := os.WriteFile(path, []byte("x\n"), 0o644); err != nil {
-			t.Fatal(err)
-		}
-	}
-	skill, ok := Get(root, "many")
-	if !ok {
-		t.Fatal("skill not loaded")
-	}
-	output := ToolOutput(skill)
-	if got := strings.Count(output, "\n- references/"); got != bundledFileListLimit {
-		t.Fatalf("listed %d files, want the first %d:\n%s", got, bundledFileListLimit, output)
-	}
-	if !strings.Contains(output, "- (and 7 more)\n") {
-		t.Fatalf("output does not say how many more there are:\n%s", output)
-	}
-}
-
-func TestToolOutputOfAManifestOnlySkillIsItsContent(t *testing.T) {
-	root := t.TempDir()
-	writeSourceSkill(t, filepath.Join(root, "plain"), "---\nname: plain\n---\nJust instructions.\n")
-	skill, ok := Get(root, "plain")
-	if !ok {
-		t.Fatal("skill not loaded")
-	}
-	if got := ToolOutput(skill); got != skill.Content {
-		t.Fatalf("ToolOutput = %q, want the unchanged content %q", got, skill.Content)
-	}
-}

internal/skills/export_test.go

@@ -15,10 +15,9 @@ func Info(dir string, name string) (SkillInfo, bool) {
 		if entry, found := lock[skill.Name]; found {
 			info.Source = entry.Source
 			info.Hash = entry.Hash
-			info.TreeHash = entry.TreeHash
 		}
 	}
-	info.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
+	info.HashDrift = skillHashDrift(skill, info.Hash)
 	return info, true
 }
 

internal/skills/install.go

@@ -1,9 +1,9 @@
 package skills
 
 // Distribution: install a skill from a git URL or a local path into the skills
-// directory, with content hashes recorded in a lockfile (skills.lock) so every
-// install/update is verifiable. Install copies the skill's directory, its
-// SKILL.md and the files beside it, and NEVER executes fetched content.
+// directory, with a content hash recorded in a lockfile (skills.lock) so every
+// install/update is verifiable. Skills are markdown, so install NEVER executes
+// fetched content — it copies and validates the SKILL.md and nothing else.
 
 import (
 	"context"
@@ -61,30 +61,16 @@ type InstallResult struct {
 	Hash string `json:"hash"`
 	// Source echoes the source the skill was installed from.
 	Source string `json:"source"`
-	// TreeHash covers every file the skill ships, SKILL.md included; Hash is
-	// still the SKILL.md hash alone.
-	TreeHash string `json:"treeHash"`
 	// Updated is true when an existing install was replaced; PreviousHash then
 	// carries the prior recorded hash so a caller can show the change.
 	Updated      bool   `json:"updated"`
 	PreviousHash string `json:"previousHash,omitempty"`
-	// PreviousTreeHash is the tree hash the replaced install recorded. It is
-	// empty when that install predates tree hashes, and so copied SKILL.md
-	// alone.
-	PreviousTreeHash string `json:"previousTreeHash,omitempty"`
-	// SkippedLinks lists symlinks in the source, relative to the skill
-	// directory. Install neither follows nor copies them.
-	SkippedLinks []string `json:"skippedLinks,omitempty"`
 }
 
 // LockEntry records the source and content hash for one installed skill.
 type LockEntry struct {
 	Source string `json:"source"`
 	Hash   string `json:"hash"`
-	// TreeHash covers the whole installed skill directory. Entries written
-	// before skills installed their bundled files have none, and are checked on
-	// their SKILL.md alone.
-	TreeHash string `json:"treeHash,omitempty"`
 }
 
 // SkillInfo bundles a discovered skill with its recorded source and hash, for
@@ -93,14 +79,12 @@ type SkillInfo struct {
 	Skill     Skill  `json:"skill"`
 	Source    string `json:"source,omitempty"`
 	Hash      string `json:"hash,omitempty"`
-	TreeHash  string `json:"treeHash,omitempty"`
 	HashDrift bool   `json:"hashDrift"`
 }
 
-// Install fetches the skill at options.Source and copies its directory into
-// options.Dir/<name>/: SKILL.md and the scripts, references and assets beside
-// it. It validates the frontmatter and records the SKILL.md hash and a hash of
-// the whole tree in the lockfile. A git URL is fetched via the (injectable) GitRunner into a
+// Install fetches the skill at options.Source and copies its SKILL.md into
+// options.Dir/<name>/, validating the frontmatter and recording a content hash
+// in the lockfile. A git URL is fetched via the (injectable) GitRunner into a
 // temp dir; a local path is read in place. Fetched content is never executed.
 func Install(ctx context.Context, options InstallOptions) (InstallResult, error) {
 	source := strings.TrimSpace(options.Source)
@@ -149,11 +133,10 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 	if err := os.MkdirAll(staged, 0o755); err != nil {
 		return InstallResult{}, fmt.Errorf("create staged skill dir: %w", err)
 	}
-	bundled, skippedLinks, err := copySkillTree(skillDir, staged)
-	if err != nil {
-		return InstallResult{}, fmt.Errorf("stage skill directory: %w", err)
-	}
 	stagedManifest := filepath.Join(staged, skillFileName)
+	if err := os.WriteFile(stagedManifest, data, 0o644); err != nil {
+		return InstallResult{}, fmt.Errorf("stage SKILL.md: %w", err)
+	}
 	stagedData, err := os.ReadFile(stagedManifest)
 	if err != nil || hashContent(stagedData) != hash {
 		if err != nil {
@@ -186,13 +169,8 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 		return InstallResult{}, fmt.Errorf("%w: %q is installed from %s (use --force to overwrite)", ErrNameClash, name, previous.Source)
 	}
 
-	treeHash, err := skillTreeHash(staged)
-	if err != nil {
-		return InstallResult{}, fmt.Errorf("hash staged skill directory: %w", err)
-	}
-
 	target := filepath.Join(dir, name)
-	lock[name] = LockEntry{Source: source, Hash: hash, TreeHash: treeHash}
+	lock[name] = LockEntry{Source: source, Hash: hash}
 	if err := installtxn.CommitDir(target, staged, func() error {
 		return writeLock(dir, lock)
 	}); err != nil {
@@ -200,26 +178,14 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 	}
 
 	result := InstallResult{
-		Name:         name,
-		Path:         filepath.Join(target, skillFileName),
-		Hash:         hash,
-		TreeHash:     treeHash,
-		Source:       source,
-		SkippedLinks: skippedLinks,
+		Name:   name,
+		Path:   filepath.Join(target, skillFileName),
+		Hash:   hash,
+		Source: source,
 	}
 	if existed {
+		result.Updated = previous.Hash != hash
 		result.PreviousHash = previous.Hash
-		result.PreviousTreeHash = previous.TreeHash
-		if previous.TreeHash == "" {
-			// An entry from before tree hashes is from an install that copied
-			// SKILL.md alone. Against that, this install is an update when
-			// SKILL.md changed or it now brings bundled files; a skill that is
-			// still only its unchanged SKILL.md is not.
-			result.Updated = previous.Hash != hash || bundled > 0
-		} else {
-			// A changed script with the same SKILL.md is still an update.
-			result.Updated = previous.Hash != hash || previous.TreeHash != treeHash
-		}
 	}
 	return result, nil
 }
@@ -227,8 +193,8 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 // lockReconciler answers, for one interrupted workspace, which of the two trees
 // beside each other the lockfile records. A kill between the tree swap and the
 // lockfile publish leaves exactly what a kill after both leaves, so filesystem
-// shape cannot tell them apart and only the recorded hashes can: the tree they
-// describe is the one the user was told they had.
+// shape cannot tell them apart and only the recorded hash can: the tree it
+// describes is the one the user was told they had.
 func lockReconciler(dir string) installtxn.Reconciler {
 	return func(name string, target string, backup string) (installtxn.Phase, error) {
 		lock, err := ReadLock(dir)
@@ -249,45 +215,24 @@ func lockReconciler(dir string) installtxn.Reconciler {
 			// recorded one and recovery must not pick between them.
 			return installtxn.PhaseUnknown, nil
 		}
-		committed, err := treeMatchesEntry(target, entry)
+		targetHash, err := manifestHash(target)
 		if err != nil {
 			return installtxn.PhaseUnknown, err
 		}
-		if committed {
+		if targetHash == entry.Hash {
 			return installtxn.PhaseCommitted, nil
 		}
-		prePublish, err := treeMatchesEntry(backup, entry)
+		backupHash, err := manifestHash(backup)
 		if err != nil {
 			return installtxn.PhaseUnknown, err
 		}
-		if prePublish {
+		if backupHash == entry.Hash {
 			return installtxn.PhasePrePublish, nil
 		}
 		return installtxn.PhaseUnknown, nil
 	}
 }
 
-// treeMatchesEntry reports whether the skill tree at path is the one entry
-// records. A reinstall that changes only a bundled file leaves SKILL.md the same
-// in both trees of an interrupted workspace, so an entry with a tree hash has to
-// match on that as well. An entry from before tree hashes records SKILL.md
-// alone, and that is all it can be matched on. As in manifestHash, a tree with
-// no SKILL.md is a non-match and any other read error is returned.
-func treeMatchesEntry(path string, entry LockEntry) (bool, error) {
-	sum, err := manifestHash(path)
-	if err != nil || sum != entry.Hash {
-		return false, err
-	}
-	if entry.TreeHash == "" {
-		return true, nil
-	}
-	tree, err := skillTreeHash(path)
-	if err != nil {
-		return false, err
-	}
-	return tree == entry.TreeHash, nil
-}
-
 // manifestHash hashes the SKILL.md under path the same way an install records
 // it. A tree with no SKILL.md is a non-match rather than a failure: an install
 // killed mid-copy can leave one, and it simply is not the tree the lockfile
@@ -401,10 +346,9 @@ func InfoFromRoots(primaryDir string, roots []string, name string) (SkillInfo, b
 		if entry, found := lock[skill.Name]; found {
 			info.Source = entry.Source
 			info.Hash = entry.Hash
-			info.TreeHash = entry.TreeHash
 		}
 	}
-	info.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
+	info.HashDrift = skillHashDrift(skill, info.Hash)
 	return info, true
 }
 
@@ -612,12 +556,11 @@ func hashContent(data []byte) string {
 	return "sha256:" + hex.EncodeToString(sum[:])
 }
 
-// skillHashDrift reports whether the installed skill no longer matches the
-// lockfile hashes recorded at install time: the SKILL.md hash, and the tree
-// hash when the entry has one, so an edited script counts as drift too. Missing
-// lock hashes never count as drift (skills Zero did not install). A locked
-// skill that cannot be read is treated as drifted, not as a clean match.
-func skillHashDrift(skill Skill, lockHash string, lockTreeHash string) bool {
+// skillHashDrift reports whether the on-disk SKILL.md no longer matches the
+// lockfile hash recorded at install time. Missing lock hashes never count as
+// drift (agents-only / unlocked skills). A locked skill whose SKILL.md cannot
+// be read is treated as drifted — not as a clean match.
+func skillHashDrift(skill Skill, lockHash string) bool {
 	lockHash = strings.TrimSpace(lockHash)
 	if lockHash == "" {
 		return false
@@ -626,13 +569,5 @@ func skillHashDrift(skill Skill, lockHash string, lockTreeHash string) bool {
 	if err != nil {
 		return true
 	}
-	if hashContent(data) != lockHash {
-		return true
-	}
-	lockTreeHash = strings.TrimSpace(lockTreeHash)
-	if lockTreeHash == "" {
-		return false
-	}
-	treeHash, err := skillTreeHash(filepath.Dir(skill.Path))
-	return err != nil || treeHash != lockTreeHash
+	return hashContent(data) != lockHash
 }

internal/skills/install_test.go

@@ -364,11 +364,10 @@ func TestInstallGitSourceUsesRunnerAndDoesNotExecuteContent(t *testing.T) {
 	if _, err := os.Stat(filepath.Join(cloneRoot, "PWNED")); err == nil {
 		t.Fatalf("install must never execute fetched content")
 	}
-	// A script beside SKILL.md is part of the skill (#584), so it is installed,
-	// and still only copied: running it is up to whoever uses the skill later,
-	// through the normal tool permissions.
-	if _, err := os.Stat(filepath.Join(destDir, "remote", "install.sh")); err != nil {
-		t.Fatalf("the skill's bundled script was not installed: %v", err)
+	// The fetched executable must not be copied into the skills dir either (skills
+	// are markdown — only SKILL.md is installed, not arbitrary fetched files).
+	if _, err := os.Stat(filepath.Join(destDir, "remote", "install.sh")); err == nil {
+		t.Fatalf("install must not copy fetched executable into the skills dir")
 	}
 }
 
@@ -441,10 +440,10 @@ func TestInfoReturnsFrontmatterSourceAndHash(t *testing.T) {
 
 func TestSkillHashDriftUnreadableLockedPath(t *testing.T) {
 	missing := filepath.Join(t.TempDir(), "missing", "SKILL.md")
-	if !skillHashDrift(Skill{Path: missing}, "sha256:deadbeef", "") {
+	if !skillHashDrift(Skill{Path: missing}, "sha256:deadbeef") {
 		t.Fatal("expected drift when locked SKILL.md cannot be read")
 	}
-	if skillHashDrift(Skill{Path: missing}, "", "") {
+	if skillHashDrift(Skill{Path: missing}, "") {
 		t.Fatal("missing lock hash must not count as drift")
 	}
 }
@@ -862,146 +861,6 @@ func TestRecoveryResolvesEveryInterruptedUpdateStep(t *testing.T) {
 	}
 }
 
-const (
-	bundledScriptV1 = "print('v1')\n"
-	bundledScriptV2 = "print('v2')\n"
-)
-
-func writeBundledScript(t *testing.T, skillDir string, content string) string {
-	t.Helper()
-	path := filepath.Join(skillDir, "scripts", "run.py")
-	if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(path, []byte(content), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	return path
-}
-
-// plantInterruptedBundledUpdate is plantInterruptedAlphaUpdate for a reinstall
-// that changes only a bundled script, killed after the tree swap: alpha's
-// SKILL.md is the same in the live tree and the backup, and only the tree hash
-// in the lockfile tells them apart. published says whether the lockfile was
-// rewritten for the new tree before the kill (S4) or not (S3).
-func plantInterruptedBundledUpdate(t *testing.T, published bool) interruptedUpdate {
-	t.Helper()
-	dir := t.TempDir()
-	source := writeSourceSkill(t, filepath.Join(t.TempDir(), "alpha"), oldAlphaSkill)
-	writeBundledScript(t, source, bundledScriptV1)
-	if _, err := Install(context.Background(), InstallOptions{Source: source, Dir: dir}); err != nil {
-		t.Fatalf("seed alpha: %v", err)
-	}
-	gammaSource := writeSourceSkill(t, filepath.Join(t.TempDir(), "gamma"), gammaSkill)
-	if _, err := Install(context.Background(), InstallOptions{Source: gammaSource, Dir: dir}); err != nil {
-		t.Fatalf("seed gamma: %v", err)
-	}
-
-	staged, _, err := installtxn.StageDir(dir)
-	if err != nil {
-		t.Fatal(err)
-	}
-	workspace := filepath.Dir(staged)
-	writeWorkspaceMarker(t, workspace, "alpha")
-	writeSourceSkill(t, staged, oldAlphaSkill)
-	writeBundledScript(t, staged, bundledScriptV2)
-
-	target := filepath.Join(dir, "alpha")
-	if err := os.Rename(target, filepath.Join(workspace, "previous")); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Rename(staged, target); err != nil {
-		t.Fatal(err)
-	}
-	if published {
-		treeHash, err := skillTreeHash(target)
-		if err != nil {
-			t.Fatal(err)
-		}
-		publishLockEntry(t, dir, "alpha", LockEntry{Source: canonicalSource(source), Hash: hashContent([]byte(oldAlphaSkill)), TreeHash: treeHash})
-	}
-	return interruptedUpdate{dir: dir, workspace: workspace, oldSource: canonicalSource(source), newSource: canonicalSource(source)}
-}
-
-// SKILL.md cannot say which tree of an interrupted bundled-only update the
-// lockfile records, since both carry the same one. The tree hash can: before the
-// publish it still describes the backup, after it the live tree.
-func TestRecoveryTellsABundledOnlyUpdateApartByTreeHash(t *testing.T) {
-	steps := []struct {
-		step      string
-		published bool
-		want      string
-	}{
-		{step: "S3", published: false, want: bundledScriptV1},
-		{step: "S4", published: true, want: bundledScriptV2},
-	}
-	for _, step := range steps {
-		for _, driver := range recoveryDrivers {
-			t.Run(step.step+"/"+driver.name, func(t *testing.T) {
-				update := plantInterruptedBundledUpdate(t, step.published)
-
-				if err := driver.run(t, update.dir); err != nil {
-					t.Fatalf("%s over a recoverable state: %v", driver.name, err)
-				}
-
-				target := filepath.Join(update.dir, "alpha")
-				got, err := os.ReadFile(filepath.Join(target, "scripts", "run.py"))
-				if err != nil || string(got) != step.want {
-					t.Fatalf("recovered script = %q, %v; want %q", got, err, step.want)
-				}
-				lock, err := ReadLock(update.dir)
-				if err != nil {
-					t.Fatal(err)
-				}
-				tree, err := skillTreeHash(target)
-				if err != nil {
-					t.Fatal(err)
-				}
-				if lock["alpha"].TreeHash != tree {
-					t.Errorf("lock tree hash %q does not describe the recovered tree %q", lock["alpha"].TreeHash, tree)
-				}
-				if _, err := os.Stat(update.workspace); !os.IsNotExist(err) {
-					t.Errorf("a resolved workspace must be cleared, got %v", err)
-				}
-			})
-		}
-	}
-}
-
-// A live tree whose files cannot be read cannot be shown to be the one the
-// lockfile records, or not to be. Recovery reports it, as it does for a SKILL.md
-// it cannot read, and decides nothing until a later pass can read it.
-func TestRecoveryReportsALiveTreeItCannotHash(t *testing.T) {
-	for _, driver := range recoveryDrivers {
-		t.Run(driver.name, func(t *testing.T) {
-			update := plantInterruptedBundledUpdate(t, false)
-			lockBefore := readLockBytes(t, update.dir)
-			dirBefore := treeSnapshot(t, update.dir)
-			release := holdUnreadable(t, filepath.Join(update.dir, "alpha", "scripts", "run.py"))
-
-			err := driver.run(t, update.dir)
-			release()
-			if err == nil || !strings.Contains(err.Error(), "classify interrupted install alpha") {
-				t.Fatalf("%s over a live tree it cannot hash: %v, want the classification error", driver.name, err)
-			}
-			assertSnapshotUnchanged(t, dirBefore, treeSnapshot(t, update.dir))
-			if got := readLockBytes(t, update.dir); got != lockBefore {
-				t.Errorf("the lockfile was rewritten by a caller that must have aborted:\n%s", got)
-			}
-
-			// Once the tree can be read, the next pass resolves it: the lockfile
-			// still describes the backup, so the backup comes back.
-			if err := driver.run(t, update.dir); err != nil {
-				t.Fatalf("%s once the tree is readable: %v", driver.name, err)
-			}
-			got, err := os.ReadFile(filepath.Join(update.dir, "alpha", "scripts", "run.py"))
-			if err != nil || string(got) != bundledScriptV1 {
-				t.Fatalf("recovered script = %q, %v; want %q", got, err, bundledScriptV1)
-			}
-		})
-	}
-}
-
 // A recovery that could not be made is not the same as nothing to recover.
 // Installing over or reporting the removal of a tree that is still owed a
 // restore destroys the only copy of it, so the caller has to stop before it

internal/skills/unreadable_posix_test.go

@@ -1,30 +0,0 @@
-//go:build !windows
-
-package skills
-
-import (
-	"os"
-	"sync"
-	"testing"
-)
-
-// holdUnreadable takes every permission off path, so reading it fails until
-// release puts them back. Cleanup releases it too. Root reads a file whatever
-// its mode, so a test that needs the read to fail is skipped there.
-func holdUnreadable(t *testing.T, path string) (release func()) {
-	t.Helper()
-	if os.Geteuid() == 0 {
-		t.Skip("root reads a file whatever its mode")
-	}
-	info, err := os.Stat(path)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if err := os.Chmod(path, 0); err != nil {
-		t.Fatal(err)
-	}
-	var once sync.Once
-	release = func() { once.Do(func() { _ = os.Chmod(path, info.Mode().Perm()) }) }
-	t.Cleanup(release)
-	return release
-}

internal/skills/unreadable_windows_test.go

@@ -1,28 +0,0 @@
-//go:build windows
-
-package skills
-
-import (
-	"sync"
-	"testing"
-
-	"golang.org/x/sys/windows"
-)
-
-// holdUnreadable keeps path open with no sharing, so every other open of it
-// fails until release runs. Cleanup releases it too.
-func holdUnreadable(t *testing.T, path string) (release func()) {
-	t.Helper()
-	name, err := windows.UTF16PtrFromString(path)
-	if err != nil {
-		t.Fatal(err)
-	}
-	handle, err := windows.CreateFile(name, windows.GENERIC_READ, 0, nil, windows.OPEN_EXISTING, windows.FILE_ATTRIBUTE_NORMAL, 0)
-	if err != nil {
-		t.Fatalf("hold %s open: %v", path, err)
-	}
-	var once sync.Once
-	release = func() { once.Do(func() { _ = windows.CloseHandle(handle) }) }
-	t.Cleanup(release)
-	return release
-}

internal/tools/exec_command_test.go

@@ -936,15 +936,8 @@ func TestExecCommandInteractiveBlockSuggestsTTY(t *testing.T) {
 }
 
 func TestExecCommandTTYSkipsInteractiveBlock(t *testing.T) {
-	// The ssh can still be resolving the bogus host when the 250ms yield
-	// returns, which leaves its session running in the test root. Stop it
-	// before the root goes, and use execTestRoot for the moment Windows still
-	// holds the directory after the stop.
-	root := execTestRoot(t)
-	manager := newExecSessionManager()
-	t.Cleanup(func() { manager.StopAll() })
-	tool := NewScopedExecCommandTool(root, nil, manager)
-	// ssh to a bogus host fails on its own; the point is that the
+	tool := NewScopedExecCommandTool(t.TempDir(), nil, newExecSessionManager())
+	// ssh to a bogus host fails fast on its own; the point is that the
 	// interactive guard must not fire ahead of the tty path.
 	result := tool.Run(context.Background(), map[string]any{
 		"cmd": "ssh host.example.com", "tty": true, "yield_time_ms": 250,

internal/tools/skill.go

@@ -73,7 +73,7 @@ func (tool *skillTool) Run(_ context.Context, args map[string]any) Result {
 	names := make([]string, 0, len(loaded))
 	for _, skill := range loaded {
 		if skill.Name == name {
-			return okResult(skills.ToolOutput(skill))
+			return okResult(skill.Content)
 		}
 		names = append(names, skill.Name)
 	}

internal/tools/skill_test.go

@@ -96,27 +96,3 @@ func TestSkillToolNoSkillsAvailable(t *testing.T) {
 		t.Fatalf("expected a no-skills message, got: %q", result.Output)
 	}
 }
-
-// A skill that ships files beside its SKILL.md (#584) is no use if the model
-// cannot find them, so the output names their directory and lists them.
-func TestSkillToolPointsAtTheSkillsBundledFiles(t *testing.T) {
-	dir := t.TempDir()
-	writeSkillFile(t, dir, "pdf", "---\nname: pdf\n---\nRun scripts/fill.py.")
-	script := filepath.Join(dir, "pdf", "scripts", "fill.py")
-	if err := os.MkdirAll(filepath.Dir(script), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(script, []byte("print('fill')\n"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-
-	result := NewSkillTool(dir).Run(context.Background(), map[string]any{"name": "pdf"})
-	if result.Status != StatusOK {
-		t.Fatalf("Status = %s (output: %s)", result.Status, result.Output)
-	}
-	for _, want := range []string{"Run scripts/fill.py.", filepath.Join(dir, "pdf"), "- scripts/fill.py"} {
-		if !strings.Contains(result.Output, want) {
-			t.Fatalf("output does not contain %q:\n%s", want, result.Output)
-		}
-	}
-}

internal/tui/ask_user_test.go

@@ -23,7 +23,7 @@ func testAskUserRequest() agent.AskUserRequest {
 
 func newAskUserModel(t *testing.T, request agent.AskUserRequest, answers *[][]string) model {
 	t.Helper()
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m.pending = true
 	m.activeRunID = 7
 	m.width = 96

internal/tui/autocomplete_test.go

@@ -306,7 +306,7 @@ func TestSuggestionsSuppressedDuringSpecReview(t *testing.T) {
 }
 
 func TestSuggestionOverlayRenders(t *testing.T) {
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m.width, m.height = 96, 30
 	m = typeRunes(t, m, "/mo")
 
@@ -387,7 +387,7 @@ func TestOverlayViewportLinesPreservesTextOutsidePanel(t *testing.T) {
 }
 
 func TestSuggestionOverlayCapsRowsWithoutMoreText(t *testing.T) {
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m.width, m.height = 96, 30
 	m = typeRunes(t, m, "/")
 

internal/tui/commands_test.go

@@ -263,7 +263,7 @@ func TestMCPManagerNavigationOpensAddWizard(t *testing.T) {
 }
 
 func TestMCPManagerSearchFiltersMarketplace(t *testing.T) {
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m.width = 120
 	m.height = 36
 	m.input.SetValue("/mcp")

internal/tui/hermetic_test.go

@@ -4,100 +4,12 @@ import (
 	"context"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 
 	"github.com/Twigpine/zero/internal/config"
 	"github.com/Twigpine/zero/internal/testutil"
 )
 
-// newViewTestModel builds the model for a test that asserts on the whole view.
-// With an empty Options.Cwd, newModel uses this package's directory, so the
-// title bar shows the real branch and checkout path, and a check that some text
-// is absent fails wherever either one contains it: a branch named
-// feat/model-picker breaks a "/model" check, a checkout under ~/more breaks a
-// "more" check.
-//
-// The cwd is isolatedCwd, so the path in the title bar is the same on every
-// machine. The branch is cleared after construction, because no path can
-// promise there is none: gitBranch walks up from the cwd even though it does
-// not exist, so a temporary directory inside a checkout would still put that
-// checkout's branch in the title bar.
-func newViewTestModel(t *testing.T, options Options) model {
-	t.Helper()
-	options.Cwd = isolatedCwd(t)
-	m := newModel(context.Background(), options)
-	m.gitBranch = ""
-	return m
-}
-
-// isolatedCwd is a child of the home TestMain isolates that nothing creates:
-// fully qualified on every platform and holding nothing newModel could read.
-// The title bar shows it as ~/not-a-checkout. A t.TempDir path would carry the
-// user name and the test name into the view instead.
-func isolatedCwd(t *testing.T) string {
-	t.Helper()
-	home, err := os.UserHomeDir()
-	if err != nil {
-		t.Fatal(err)
-	}
-	return filepath.Join(home, "not-a-checkout")
-}
-
-// Guards isolatedCwd: the view tests rely on a fully qualified path that does
-// not exist. Whether a checkout encloses it depends on where the temporary
-// directory is, which is why newViewTestModel clears the branch instead.
-func TestIsolatedCwdIsAbsentAndFullyQualified(t *testing.T) {
-	cwd := isolatedCwd(t)
-	if !filepath.IsAbs(cwd) {
-		t.Fatalf("isolatedCwd = %q, want a fully qualified path", cwd)
-	}
-	if _, err := os.Lstat(cwd); !os.IsNotExist(err) {
-		t.Fatalf("isolatedCwd %q must not exist, got %v", cwd, err)
-	}
-}
-
-// A temporary directory inside a checkout puts the home, and so isolatedCwd,
-// under that checkout. Its branch must still stay out of the view, or a branch
-// named feat/model-picker fails the "/model" check again.
-func TestViewTestModelKeepsAnEnclosingCheckoutsBranchOutOfTheView(t *testing.T) {
-	checkout := t.TempDir()
-	if err := os.MkdirAll(filepath.Join(checkout, ".git"), 0o755); err != nil {
-		t.Fatal(err)
-	}
-	if err := os.WriteFile(filepath.Join(checkout, ".git", "HEAD"), []byte("ref: refs/heads/feat/model-picker\n"), 0o644); err != nil {
-		t.Fatal(err)
-	}
-	home := filepath.Join(checkout, "home")
-	t.Setenv("HOME", home)
-	t.Setenv("USERPROFILE", home)
-	if got := gitBranch(isolatedCwd(t)); got != "feat/model-picker" {
-		t.Fatalf("fixture: gitBranch(isolatedCwd) = %q, want the enclosing checkout's feat/model-picker", got)
-	}
-
-	m := newViewTestModel(t, Options{})
-	m.width, m.height = 96, 30
-	m = typeRunes(t, m, "/mo")
-	plain := plainRender(t, m.View())
-	if strings.Contains(plain, "feat/model-picker") || strings.Contains(plain, "/model") {
-		t.Fatalf("the enclosing checkout's branch reached the view:\n%s", plain)
-	}
-	// The path is the other half: the title bar shows the isolated cwd, not the
-	// checkout the tests happen to run from or the one enclosing the home.
-	if !strings.Contains(plain, "not-a-checkout") {
-		t.Fatalf("the title bar does not show the isolated cwd:\n%s", plain)
-	}
-	wd, err := os.Getwd()
-	if err != nil {
-		t.Fatal(err)
-	}
-	for _, path := range []string{checkout, wd} {
-		if strings.Contains(plain, path) {
-			t.Fatalf("the view shows the checkout path %s:\n%s", path, plain)
-		}
-	}
-}
-
 // Guards TestMain: the per-user directories every newModel reads (and the ones
 // tests could write to) must resolve inside the isolated root on every OS.
 func TestUserDirsAreIsolatedFromTheDeveloperHome(t *testing.T) {

internal/tui/model_test.go

@@ -357,7 +357,7 @@ func TestInitialRenderShowsLimeChatSurface(t *testing.T) {
 }
 
 func TestEmptyStateCollapsesAfterFirstPrompt(t *testing.T) {
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m.width = 100
 	m.height = 30
 	m.input.SetValue("inspect the repo")

internal/tui/provider_wizard_test.go

@@ -562,7 +562,7 @@ func TestProviderWizardCustomCompatibleProviderDerivesIPName(t *testing.T) {
 }
 
 func TestProviderWizardSkipsAPIKeyForLocalProvidersAndEscCloses(t *testing.T) {
-	m := newViewTestModel(t, Options{})
+	m := newModel(context.Background(), Options{})
 	m = openProviderWizardForTest(t, m)
 	m.providerWizard.selectedProvider = providerWizardProviderIndex(t, m.providerWizard, "ollama")
 
@@ -773,7 +773,7 @@ func TestProviderWizardUsesAPIKeyEnvForCurrentSessionWithoutPersistingSecret(t *
 
 func TestProviderWizardUsesLiveDiscoveredModels(t *testing.T) {
 	var captured config.ProviderProfile
-	m := newViewTestModel(t, Options{
+	m := newModel(context.Background(), Options{
 		DiscoverProviderModels: func(ctx context.Context, profile config.ProviderProfile) ([]providermodeldiscovery.Model, error) {
 			captured = profile
 			return []providermodeldiscovery.Model{{ID: "live-b"}, {ID: "live-a"}}, nil
@@ -874,7 +874,7 @@ func TestProviderWizardIgnoresStaleDiscoveryForSameProvider(t *testing.T) {
 }
 
 func TestProviderWizardKeepsFallbackModelsWhenLiveDiscoveryFails(t *testing.T) {
-	m := newViewTestModel(t, Options{
+	m := newModel(context.Background(), Options{
 		DiscoverProviderModels: func(context.Context, config.ProviderProfile) ([]providermodeldiscovery.Model, error) {
 			return nil, errors.New("offline")
 		},

internal/tui/rendering_lime_test.go

@@ -1585,7 +1585,7 @@ func TestTitleBarHighlightsBranchOverWorkspace(t *testing.T) {
 }
 
 func TestGenericCustomProviderDisplayUsesEndpointName(t *testing.T) {
-	m := newViewTestModel(t, Options{
+	m := newModel(context.Background(), Options{
 		ProviderName: "custom-openai-compatible",
 		ModelName:    "MiniMax-M3",
 		ProviderProfile: config.ProviderProfile{

package-lock.json

@@ -452,9 +452,9 @@
       }
     },
     "node_modules/hono": {
-      "version": "4.13.13",
-      "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.13.tgz",
-      "integrity": "sha512-CQ46U0ZkAGmbT/4UxdzzGJpacP2IeKgY4a5/tOI9AABbpOMfK739wfDXmv1usCk+3RkKj1hQy4/fjhiwa2xlrA==",
+      "version": "4.13.9",
+      "resolved": "https://registry.npmjs.org/hono/-/hono-4.13.9.tgz",
+      "integrity": "sha512-7dMkQmZoC4E6F7AtaQSPhlWAdnBti+j7rreMZl8QB4jFiEhP9TWbGWUMi8WYzBCgmgulxuvLQupKqo+Co6Omyg==",
       "license": "MIT",
       "engines": {
         "node": ">=16.9.0"