test(tools): stop the tty session before removing its root

TestExecCommandTTYSkipsInteractiveBlock runs ssh to a bogus host under
tty with a 250ms yield. When ssh is still resolving the host after the
yield, the session keeps running in the test's directory and nothing
stops it, so on Windows t.TempDir's cleanup fails with a sharing
violation. It failed Smoke (windows-latest) on a README-only PR.

Stop the session in a cleanup, and build the tool on execTestRoot,
which retries the removal while Windows still holds the directory right
after the stop, as the other exec tests that leave a session do.

README.md

@@ -12,7 +12,7 @@
 
 <p align="center">
   <a href="LICENSE"><img alt="license" src="https://img.shields.io/badge/license-MIT-blue"></a>
-  <img alt="Go 1.26.9+" src="https://img.shields.io/badge/Go-1.26.9+-00ADD8?logo=go&logoColor=white">
+  <img alt="Go 1.26.10+" src="https://img.shields.io/badge/Go-1.26.10+-00ADD8?logo=go&logoColor=white">
   <img alt="25+ providers" src="https://img.shields.io/badge/providers-25+-34E2EA">
   <a href="https://discord.gg/CaQDS6wdFn"><img alt="Discord" src="https://img.shields.io/badge/Discord-join-5865F2?logo=discord&logoColor=white"></a>
   <br>
@@ -158,7 +158,7 @@ irm https://raw.githubusercontent.com/Twigpine/zero/main/scripts/install.ps1 | i
 
 ### From source
 
-Source builds require Go 1.26.9+.
+Source builds require Go 1.26.10+.
 
 ```bash
 git clone https://github.com/Twigpine/zero.git

README_ZH.md

@@ -12,7 +12,7 @@
 
 <p align="center">
   <a href="LICENSE"><img alt="license" src="https://img.shields.io/badge/license-MIT-blue"></a>
-  <img alt="Go 1.26.9+" src="https://img.shields.io/badge/Go-1.26.9+-00ADD8?logo=go&logoColor=white">
+  <img alt="Go 1.26.10+" src="https://img.shields.io/badge/Go-1.26.10+-00ADD8?logo=go&logoColor=white">
   <img alt="25+ providers" src="https://img.shields.io/badge/providers-25+-34E2EA">
   <a href="https://discord.gg/CaQDS6wdFn"><img alt="Discord" src="https://img.shields.io/badge/Discord-join-5865F2?logo=discord&logoColor=white"></a>
   <br>
@@ -136,7 +136,7 @@ irm https://raw.githubusercontent.com/Twigpine/zero/main/scripts/install.ps1 | i
 
 ### 从源码构建
 
-源码构建需要 Go 1.26.9+。
+源码构建需要 Go 1.26.10+。
 
 ```bash
 git clone https://github.com/Twigpine/zero.git

docs/EXTENDING.md

@@ -110,6 +110,8 @@ 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/
@@ -140,6 +142,15 @@ 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:

docs/INSTALL.md

@@ -190,7 +190,7 @@ Every release so far declares the module path `github.com/Gitlawb/zero`, so
 fails with "module declares its path as" until a release is tagged with the new
 module path.
 
-This builds from source, so it needs Go 1.26.9+ and it does not go through the
+This builds from source, so it needs Go 1.26.10+ and it does not go through the
 release archives. Two consequences worth knowing before you pick it:
 
 - On Linux, Bubblewrap is still required for native sandboxing. Zero resolves
@@ -218,7 +218,7 @@ Build a local binary:
 go build -o zero ./cmd/zero
 ```
 
-Source builds require Go 1.26.9+.
+Source builds require Go 1.26.10+.
 
 ### Sandbox Helpers For Source Builds
 

go.mod

@@ -1,6 +1,6 @@
 module github.com/Twigpine/zero
 
-go 1.26.9
+go 1.26.10
 
 require (
 	charm.land/bubbles/v2 v2.1.1

internal/cli/distribution.go

@@ -92,13 +92,25 @@ 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))
-		lines = append(lines, fmt.Sprintf("  hash: %s -> %s", shortHash(result.PreviousHash), shortHash(result.Hash)))
+		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")
+		}
 	} 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,6 +7,8 @@ import (
 	"path/filepath"
 	"strings"
 	"testing"
+
+	"github.com/Twigpine/zero/internal/skills"
 )
 
 func writeSourceSkillDir(t *testing.T, dir string, content string) string {
@@ -391,3 +393,133 @@ 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/cli/exec_streamjson_images_test.go

@@ -54,10 +54,10 @@ func TestRunExecImageAttachmentHistory(t *testing.T) {
 		{name: "stream and file images", model: "gpt-4.1", fileImage: true},
 	} {
 		t.Run(tc.name, func(t *testing.T) {
-			root := t.TempDir()
-			for _, name := range []string{"HOME", "USERPROFILE", "APPDATA", "LOCALAPPDATA", "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME"} {
-				t.Setenv(name, root)
-			}
+			// The shared isolation also turns off the models.dev refresh that
+			// runExec starts in the background. Without it, the refresh downloads
+			// the catalog into this HOME and races the TempDir cleanup.
+			isolateCLIUserState(t)
 			cwd := t.TempDir()
 			png := []byte{0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0, 0, 0, 0x0d, 0x49, 0x48, 0x44, 0x52}
 			encoded := base64.StdEncoding.EncodeToString(png)
@@ -148,6 +148,7 @@ func TestRunExecImageAttachmentHistory(t *testing.T) {
 // user turn). Before the fix, stream-json images were parsed and validated but
 // silently dropped — never threaded into the agent run.
 func TestRunExecStreamJSONImageReachesAgent(t *testing.T) {
+	isolateCLIUserState(t)
 	cwd := t.TempDir()
 
 	// A minimal PNG, base64-encoded for the stream-json image payload.
@@ -217,6 +218,7 @@ func TestRunExecStreamJSONImageReachesAgent(t *testing.T) {
 // the fix, ResolvePrompt's "must include at least one prompt or user message
 // event" error was returned before images were considered, rejecting the turn.
 func TestRunExecStreamJSONImageOnlyMessageProceeds(t *testing.T) {
+	isolateCLIUserState(t)
 	cwd := t.TempDir()
 
 	png := []byte{

internal/modelregistry/modelsdev_test.go

@@ -149,6 +149,7 @@ func TestRefreshModelsDevCacheRejectsBadBodyWithoutClobbering(t *testing.T) {
 	}
 	t.Setenv("ZERO_MODELS_CACHE_PATH", cachePath)
 	t.Setenv("ZERO_MODELS_URL", server.URL)
+	t.Setenv("ZERO_DISABLE_MODELS_FETCH", "")
 
 	if err := RefreshModelsDevCache(t.Context()); err == nil {
 		t.Fatal("bad body must return an error")
@@ -211,6 +212,9 @@ func TestCachedModelsDevProvidersIgnoresStaleCache(t *testing.T) {
 		t.Fatal(err)
 	}
 	t.Setenv("ZERO_MODELS_CACHE_PATH", cachePath)
+	// The variable turns the overlay off too, which would pass this test
+	// without the staleness check ever running.
+	t.Setenv("ZERO_DISABLE_MODELS_FETCH", "")
 	resetModelsDevCacheForTest()
 	t.Cleanup(resetModelsDevCacheForTest)
 	EnableModelsDevOverlay()
@@ -226,6 +230,9 @@ func TestCachedModelsDevProvidersRequiresOptIn(t *testing.T) {
 		t.Fatal(err)
 	}
 	t.Setenv("ZERO_MODELS_CACHE_PATH", cachePath)
+	// As above: with the variable set, a nil result says nothing about the
+	// opt-in.
+	t.Setenv("ZERO_DISABLE_MODELS_FETCH", "")
 	resetModelsDevCacheForTest()
 	t.Cleanup(resetModelsDevCacheForTest)
 
@@ -242,6 +249,7 @@ func TestDefaultModelEntriesAppliesFreshCache(t *testing.T) {
 		t.Fatal(err)
 	}
 	t.Setenv("ZERO_MODELS_CACHE_PATH", cachePath)
+	t.Setenv("ZERO_DISABLE_MODELS_FETCH", "")
 	resetModelsDevCacheForTest()
 	t.Cleanup(resetModelsDevCacheForTest)
 	EnableModelsDevOverlay()

internal/oauth/encrypt.go

@@ -1,191 +0,0 @@
-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,103 +1,13 @@
 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")
@@ -202,3 +112,36 @@ 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

@@ -0,0 +1,98 @@
+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

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

internal/oauth/secret_access_windows.go

@@ -1,14 +0,0 @@
-//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,7 +18,9 @@ import (
 	"sync"
 	"time"
 
+	"github.com/Twigpine/zero/internal/fsutil"
 	"github.com/Twigpine/zero/internal/keyring"
+	"github.com/Twigpine/zero/internal/securefile"
 )
 
 const (
@@ -155,7 +157,7 @@ const (
 // is AES-256-GCM ciphertext at rest.
 type Store struct {
 	blob    blobStore
-	crypter *aesGCMCrypter // nil => plaintext blob
+	crypter *securefile.Crypter // nil => plaintext blob
 	now     func() time.Time
 	mu      sync.Mutex
 }
@@ -244,8 +246,15 @@ 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 (see encrypt.go).
-		return &Store{blob: fileBlob{path: path}, crypter: newAESGCMCrypter(path + ".secret"), now: now}, nil
+		// 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
 	case "keyring":
 		kr := options.Keyring
 		if kr == nil {
@@ -405,7 +414,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)
 		}
@@ -437,9 +446,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 err
+			return fmt.Errorf("oauth: encrypt token store: %w", err)
 		}
 	}
 	return s.blob.write(payload)
@@ -511,6 +520,31 @@ 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
@@ -524,10 +558,20 @@ func (b fileBlob) write(data []byte) error {
 		_ = temp.Close()
 		return err
 	}
-	if err := temp.Close(); err != nil {
-		return err
+	if err := syncOAuthFileFn(temp); err != nil {
+		_ = temp.Close()
+		return fmt.Errorf("oauth: sync token store: %w", err)
 	}
-	return os.Rename(tempPath, b.path)
+	if err := temp.Close(); err != nil {
+		return fmt.Errorf("oauth: write token store: %w", 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
 }
 
 // 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: skill.Content}
+			return tools.Result{Status: tools.StatusOK, Output: skills.ToolOutput(skill)}
 		}
 		available = append(available, skill.Name)
 	}

internal/plugins/activate_test.go

@@ -673,3 +673,28 @@ 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/redaction/redaction.go

@@ -112,6 +112,42 @@ var textSecretPrefixes = [][]string{
 	{"eyJ"},
 }
 
+type secretTrigger struct {
+	literal string
+	// shape indexes the anchoredSecretPatterns entry the trigger belongs to,
+	// so a cut can chain the bytes after it against the shape the trigger
+	// actually starts.
+	shape int
+}
+
+// secretTriggers flattens textSecretPrefixes into one sorted list for the
+// swallowed-trigger search below. Sorted so cuts are discovered earliest-first
+// regardless of which pattern list they came from.
+var secretTriggers = func() []secretTrigger {
+	pairs := []secretTrigger{}
+	for shape, prefixes := range textSecretPrefixes {
+		for _, prefix := range prefixes {
+			pairs = append(pairs, secretTrigger{literal: prefix, shape: shape})
+		}
+	}
+	sort.Slice(pairs, func(i, j int) bool { return pairs[i].literal < pairs[j].literal })
+	return pairs
+}()
+
+// longestTrigger bounds how far past a swallowed-scored span the trigger
+// inspection may need to look: an occurrence whose last bytes run past the
+// span end (a greedy body stops at a byte outside its class, and several
+// triggers end in one).
+var longestTrigger = func() int {
+	longest := 0
+	for _, trigger := range secretTriggers {
+		if len(trigger.literal) > longest {
+			longest = len(trigger.literal)
+		}
+	}
+	return longest
+}()
+
 // anchoredSecretPatterns mirrors textSecretPatterns with the leading \b
 // replaced by \A, so each shape can be tested at an exact byte offset. A
 // credential that starts exactly where a redacted span ends (two AWS keys
@@ -129,52 +165,226 @@ var anchoredSecretPatterns = func() []*regexp.Regexp {
 	return out
 }()
 
-// redactAdjacent redacts every match of pattern, plus any match of anchored
-// that begins exactly where a redacted span ends. This catches credentials
-// glued end to end (AKIA...AKIA...) without weakening the leading-boundary
-// rule elsewhere: a mid-word occurrence that does not abut a redacted span
-// still does not match. Chained runs (three or more glued credentials) are
-// followed to the end.
+// A credential's body can run into the trigger prefix of the NEXT one when
+// both are glued end to end. For fixed-length bodies this cannot happen — the
+// `{16}` stops the body exactly where the next AKIA...AKIA... key begins,
+// which is the case the anchored end-chaining always covered. An unbounded
+// body ({36,} on a github key, {20,} on an API key) swallows whichever trigger
+// bytes sit inside its class — "ghp" after a github key is all lowercase
+// letters — so the first match ends PAST the next credential's trigger, and a
+// scan that re-anchors at the span end finds nothing: the second credential
+// is never redacted.
+//
+// redactedSpan is one region of a redaction: the slice s[start:end] and the
+// bytes to write in its place. Chained runs can link shapes together, so a
+// span says which shape produced it rather than assuming the caller's pattern
+// covered it.
+type redactedSpan struct {
+	start int
+	end   int
+	// text replaces s[start:end]. Every entry in textSecretPatterns captures
+	// nothing, so this is the replacement itself unless a custom replacement
+	// template references a group; anchoring the expansion to the shape that
+	// matched keeps that working for cross-shape chains.
+	text string
+}
+
+// redactAdjacent redacts every match of pattern, plus every credential glued
+// into the runs those matches start. Runs follow three loss-of-boundary
+// mechanisms: end-chaining (a credential glued exactly at the previous span's
+// end, the AKIA...AKIA... case), all-shape chaining (a glued credential of a
+// different shape), and the swallowed-trigger cut (one credential's greedy
+// body runs into the next one's trigger prefix, hiding it from any scan that
+// re-anchors at a span end). A run ends at the first byte boundary where no
+// shape matches; a mid-word lookalike never matches and never starts one.
 func redactAdjacent(s string, pattern, anchored *regexp.Regexp, replacement string) string {
-	spans := pattern.FindAllStringIndex(s, -1)
-	// Bound the outer loop to the original match count. The inner loop
-	// appends chained spans to the same slice; re-reading len(spans) would
-	// re-chain every appended span, growing the span count (and the
-	// FindStringIndex calls) exponentially on long glued runs.
-	n := len(spans)
-	for i := 0; i < n; i++ {
-		end := spans[i][1]
-		for end < len(s) {
-			loc := anchored.FindStringIndex(s[end:])
-			if loc == nil || loc[0] != 0 || loc[1] <= 0 {
-				break
-			}
-			newEnd := end + loc[1]
-			spans = append(spans, []int{end, newEnd})
-			end = newEnd
-		}
+	plain := pattern.FindAllStringIndex(s, -1)
+	if len(plain) == 0 {
+		return s
 	}
+	// Bound the outer loop to the original match count. Every span collects
+	// its own run; re-reading len(plain) after appends would re-run every
+	// appended credential and grow the span count (with the anchored scans
+	// each drags) exponentially on long glued runs.
+	n := len(plain)
+	spans := make([]redactedSpan, 0, n+4)
+	for i := 0; i < n; i++ {
+		spans = appendCredentialRun(s, plain[i][0], plain[i][1], replacement, spans)
+	}
+	return writeRedactedSpans(s, spans)
+}
+
+func writeRedactedSpans(s string, spans []redactedSpan) string {
 	if len(spans) == 0 {
 		return s
 	}
-	sort.Slice(spans, func(i, j int) bool { return spans[i][0] < spans[j][0] })
+	sort.Slice(spans, func(i, j int) bool { return spans[i].start < spans[j].start })
 	var out strings.Builder
 	pos := 0
 	for _, sp := range spans {
-		if sp[0] < pos {
+		if sp.start < pos {
 			continue // overlapping span, already covered
 		}
-		out.WriteString(s[pos:sp[0]])
-		// s[sp[0]:sp[1]] is a match (anchored spans match at \b when taken
-		// alone), so ReplaceAllString expands $ references exactly as the
-		// plain loop below would.
-		out.WriteString(pattern.ReplaceAllString(s[sp[0]:sp[1]], replacement))
-		pos = sp[1]
+		out.WriteString(s[pos:sp.start])
+		out.WriteString(sp.text)
+		pos = sp.end
 	}
 	out.WriteString(s[pos:])
 	return out.String()
 }
 
+// appendCredentialRun follows the path that begins exactly at start, whose
+// first plain match ended at end. Every step finds the shape that matches at
+// the current boundary, tries to re-bind that match at a swallowed trigger,
+// and otherwise consumes the whole match; the run ends where no shape does.
+func appendCredentialRun(s string, start, end int, replacement string, spans []redactedSpan) []redactedSpan {
+	cur := start
+	for cur < len(s) {
+		shape, matchEnd := firstShapeAt(s, cur)
+		if shape < 0 || matchEnd == cur {
+			break
+		}
+		if cutStart, cut := earliestSwallowedCut(s, cur, matchEnd, shape); cut {
+			// The bytes before the trigger are the credential that matched;
+			// from the trigger on the run continues under the next shape.
+			spans = append(spans, redactedSpan{start: cur, end: cutStart, text: shapeReplacement(shape, s[cur:cutStart], replacement)})
+			cur = cutStart
+			continue
+		}
+		spans = append(spans, redactedSpan{start: cur, end: matchEnd, text: shapeReplacement(shape, s[cur:matchEnd], replacement)})
+		cur = matchEnd
+	}
+	return spans
+}
+
+func shapeReplacement(shape int, slice, replacement string) string {
+	return anchoredSecretPatterns[shape].ReplaceAllString(slice, replacement)
+}
+
+// firstShapeAt returns the first anchored shape that matches exactly at off,
+// with the end of its match. Shapes are tried in declaration order, so the
+// strict JWT form wins over the loose one; both redact the same bytes, so the
+// tie-break only decides which regex expanded the replacement.
+func firstShapeAt(s string, off int) (shape int, end int) {
+	for shape, anchored := range anchoredSecretPatterns {
+		loc := anchored.FindStringIndex(s[off:])
+		if loc != nil && loc[0] == 0 && loc[1] > 0 {
+			return shape, off + loc[1]
+		}
+	}
+	return -1, off
+}
+
+// earliestSwallowedCut searches the matched span [start, end) of shape for a
+// trigger literal that begins a valid credential right there: the head (the
+// bytes before it) still matches the shape as a whole, and the run chained
+// from the trigger reaches past the span. The earliest such trigger is the
+// cut a boundary at that byte would have produced.
+//
+// The head requirement and the past-the-end requirement keep the cut honest:
+// a trigger spelled mid-body that does not begin a credential leaves the span
+// alone, and a cut that stayed inside the original span recovers no bytes —
+// it would only split one redaction in two.
+//
+// The candidate set is the trigger occurrences inside the span, bounded by
+// the span plus one trigger length; every occurrence of every trigger in that
+// window is a candidate, earliest-first.
+func earliestSwallowedCut(s string, start, end, shape int) (int, bool) {
+	if end-start <= 1 {
+		return 0, false
+	}
+	lookahead := end + longestTrigger
+	if lookahead > len(s) {
+		lookahead = len(s)
+	}
+	window := s[start+1 : lookahead]
+
+	// A trigger occurrence whose bytes begin inside the span may extend past
+	// its end: the greedy body stops at one byte outside its class, and
+	// several triggers end in exactly such a byte ("ghp_"'s underscore,
+	// "sk-ant-" plus a hyphen).
+	anchored := anchoredSecretPatterns[shape]
+	examined := 0
+	for _, q := range triggerOccurrences(window, start+1, end) {
+		if examined >= maxTriggerOccurrences {
+			// Occurrences are earliest-first, so the ones past the cap are the
+			// farthest into the span; a cut among them is missed and the
+			// greedy span stands, as it did before the cut existed.
+			break
+		}
+		examined++
+		// The bytes before the trigger must still be a complete match of this
+		// shape: a trigger spelling inside a body whose head does not
+		// validate is prose-shaped, not a swallowed credential.
+		loc := anchored.FindStringIndex(s[start:q])
+		if loc == nil || loc[0] != 0 || loc[1] != q-start {
+			continue
+		}
+		if runReach(s, q) > end {
+			return q, true
+		}
+	}
+	return 0, false
+}
+
+// maxTriggerOccurrences bounds how many trigger occurrences one span is
+// examined for, after sorting. Real glued credentials announce their trigger
+// within reach of the span's own start, so a distant lookalike run does not
+// get walked to its end; the split path makes the same trade ("at most two
+// passes and no more"). A cut found past the cap is missed and the greedy
+// span stands, as it did before the cut existed.
+const maxTriggerOccurrences = 64
+
+// triggerOccurrences returns every occurrence of every trigger literal that
+// begins inside [windowStart, windowEnd), where window is the slice rooted at
+// windowStart that the caller bounded so occurrences could reach one trigger
+// length past its end. Earliest first, duplicates collapsed.
+func triggerOccurrences(window string, windowStart, windowEnd int) []int {
+	occurrences := make([]int, 0, 4)
+	for _, trigger := range secretTriggers {
+		at := 0
+		for {
+			found := strings.Index(window[at:], trigger.literal)
+			if found < 0 {
+				break
+			}
+			q := windowStart + at + found
+			if q >= windowEnd {
+				break // later occurrences for this trigger start even later
+			}
+			occurrences = append(occurrences, q)
+			at += found + 1
+		}
+	}
+	sort.Ints(occurrences)
+	// In-place deduplicate: duplicates come only from triggers shared between
+	// shapes ("eyJ"), and zero is never an occurrence — the scan starts past it.
+	unique := occurrences[:0]
+	for i, q := range occurrences {
+		if i > 0 && q == occurrences[i-1] {
+			continue
+		}
+		unique = append(unique, q)
+	}
+	return unique
+}
+
+// runReach reports how far the credential run beginning exactly at off
+// extends: each boundary picks the first shape that matches there, until one
+// does not. The run is followed, not redacted; this is the shared-size check
+// that keeps a cut honest.
+func runReach(s string, off int) int {
+	cur := off
+	for cur < len(s) {
+		_, matchEnd := firstShapeAt(s, cur)
+		if matchEnd == cur {
+			break
+		}
+		cur = matchEnd
+	}
+	return cur
+}
+
 var (
 	privateKeyPattern = regexp.MustCompile(`(?s)-----BEGIN [A-Z ]*PRIVATE KEY-----.*?-----END [A-Z ]*PRIVATE KEY-----`)
 	jsonStringPattern = regexp.MustCompile(`("([^"\\]*(?:\\.[^"\\]*)*)"\s*:\s*)"([^"\\]*(?:\\.[^"\\]*)*)"`)

internal/redaction/redaction_glued_test.go

@@ -0,0 +1,171 @@
+package redaction
+
+import (
+	"strings"
+	"testing"
+)
+
+// A GREEDY BODY SWALLOWS THE NEXT CREDENTIAL'S TRIGGER, SO THE END-CHAIN
+// NEVER SEES IT.
+//
+// The anchored chain re-binds a redacted span at its END; it can only catch a
+// credential that begins exactly where the previous one stops. That is true
+// for fixed-length bodies (AKIA{16} stops at the next key) and false for
+// unbounded ones: github's body class [A-Za-z0-9] happily eats the literal
+// "ghp" of the token glued behind it, so the first match ends past the second
+// trigger and the plain chain has nothing left to match. Every unbounded shape
+// below leaked its second credential before the swallowed-trigger cut.
+func TestRedactStringRedactsCredentialsSwallowedByAGluedRun(t *testing.T) {
+	// Assembled so no secret-shaped literal appears in source.
+	// gitleaks:allow -- synthetic redaction fixtures below
+	firstGitHub := "ghp_" + strings.Repeat("a", 36)
+	secondGitHub := "ghp_" + strings.Repeat("b", 36)
+
+	t.Run("two github tokens glued end to end", func(t *testing.T) {
+		got := RedactString(firstGitHub+secondGitHub, Options{})
+		if got != RedactedSecret+RedactedSecret {
+			t.Fatalf("second token survived a glued github pair: got %q", got)
+		}
+	})
+
+	t.Run("two JWTs glued end to end", func(t *testing.T) {
+		got := RedactString(jwtToken+otherJWT, Options{})
+		if got != RedactedSecret+RedactedSecret {
+			t.Fatalf("second token survived a glued JWT pair: got %q", got)
+		}
+	})
+
+	t.Run("third github token in the same run", func(t *testing.T) {
+		thirdGitHub := "ghp_" + strings.Repeat("c", 36)
+		got := RedactString(firstGitHub+secondGitHub+thirdGitHub, Options{})
+		if got != strings.Repeat(RedactedSecret, 3) {
+			t.Fatalf("glued github run left residue between markers: got %q", got)
+		}
+	})
+}
+
+// THE SWALLOWED TRIGGER CAN BELONG TO A DIFFERENT SHAPE. The chained run from
+// a cut must try every shape at each boundary, not only the one that matched
+// the swallowed span.
+//
+// Two outcomes are both fully redacted and the tests accept either. A github
+// body stops at the hyphen an anthropic trigger carries, so the cut recovers
+// the boundary and each key becomes its own marker. An anthropic body class
+// spans [A-Za-z0-9_-], wide enough to cover the next trigger and the
+// credential behind it whole, so the single match already consumed them and
+// the requirement is only that nothing of either key survives.
+func TestRedactStringRedactsACrossShapedCredentialGluedOntoARun(t *testing.T) {
+	// gitleaks:allow -- synthetic redaction fixtures below
+	github := "ghp_" + strings.Repeat("a", 36)
+	anthropic := "sk-ant-" + strings.Repeat("b", 20)
+	secondAnthropic := "sk-ant-" + strings.Repeat("d", 20)
+
+	// keptOut fails when any trigger or credential-shaped residue survives the
+	// redaction, the one property a glued run can never relax.
+	keptOut := func(t *testing.T, got string, pieces ...string) {
+		t.Helper()
+		for _, mark := range []string{"sk-ant-", "ghp_"} {
+			if strings.Contains(got, mark) {
+				t.Fatalf("a trigger survived a glued run: %q in %q", mark, got)
+			}
+		}
+		for _, piece := range pieces {
+			if strings.Contains(got, piece) {
+				t.Fatalf("a credential survived a glued run: parts of %q… in %q", piece[:8], got)
+			}
+		}
+	}
+
+	t.Run("anthropic glued behind github", func(t *testing.T) {
+		value := github + anthropic
+		got := RedactString(value, Options{})
+		if got != RedactedSecret+RedactedSecret {
+			t.Fatalf("anthropic key survived behind a github key: got %q", got)
+		}
+		keptOut(t, got, github, anthropic)
+	})
+
+	t.Run("anthropic glued behind anthropic, then github", func(t *testing.T) {
+		value := anthropic + secondAnthropic + github
+		got := RedactString(value, Options{})
+		if got == value {
+			t.Fatal("SETUP INVALID: the glued run is not redacted at all")
+		}
+		keptOut(t, got, anthropic, secondAnthropic, github)
+	})
+
+	t.Run("github glued behind anthropic", func(t *testing.T) {
+		value := anthropic + github
+		got := RedactString(value, Options{})
+		if got == value {
+			t.Fatal("SETUP INVALID: the glued run is not redacted at all")
+		}
+		keptOut(t, got, anthropic, github)
+	})
+}
+
+// THE CUT MUST NOT WEAKEN WHAT ALREADY WORKED.
+//
+// A trigger spelled inside a matched span that starts no credential there
+// leaves the span whole: the bytes before it do not re-validate as their
+// shape, and the run chained from it stays inside the span. The end-chaining
+// for fixed-length bodies and every single-credential behavior the suite
+// already pins must be untouched.
+func TestRedactStringSwallowedTriggerCutKeepsExistingBehavior(t *testing.T) {
+	// gitleaks:allow -- synthetic redaction fixtures below
+	github := "ghp_" + strings.Repeat("a", 36)
+
+	t.Run("two fixed-length AWS keys still chain at the end", func(t *testing.T) {
+		got := RedactString("AKIAIOSFODNN7EXAMPLE"+"ASIAIOSFODNN7EXAMPLE", Options{})
+		if got != RedactedSecret+RedactedSecret {
+			t.Fatalf("glued AWS keys regressed: got %q", got)
+		}
+	})
+
+	t.Run("an interior trigger spelling starts no credential of its own", func(t *testing.T) {
+		// A JWT payload can contain "eyJ" away from a segment start. The strict
+		// form needs the segment to open with it and the loose form needs the
+		// three-segment skeleton, so a payload fragment is prose to both and the
+		// token stays one marker.
+		token := "eyJhbGciOiJIUzI1NiJ9." + "aaaaeyJaaaaaa" + ".QWxpY2VCb2JDYXJvbA"
+		if got := RedactString(token, Options{}); got != RedactedSecret {
+			t.Fatalf("a JWT with an interior trigger spelling changed shape: got %q", got)
+		}
+	})
+
+	t.Run("a single token is one marker, not two", func(t *testing.T) {
+		got := RedactString(github, Options{})
+		if got != RedactedSecret {
+			t.Fatalf("a lone token changed shape: got %q", got)
+		}
+	})
+}
+
+// A SPAM OF TRIGGER SPELLINGS INSIDE ONE BODY MUST COST WHAT THE SPAN COSTS.
+//
+// Every trigger occurrence inside a matched span is a cut candidate, but only
+// the first maxTriggerOccurrences are examined, so the scan stays linear in
+// the span: a body stuffed with lookalikes must not walk the span once per
+// candidate. Measured as allocation against the same bar the split path's
+// linearity test uses, the one an index-per-byte or a walk-per-credential
+// explosion offends.
+func TestRedactStringTriggerLookalikeSpamStaysBounded(t *testing.T) {
+	// No digit appears anywhere in the fixture, so the openai shape's filter
+	// rejects it as kebab-case prose and the anthropic shape still matches it
+	// whole: every "sk-ant-" inside the span is a cut candidate, and none
+	// validates because the run chained from any of them reaches only to the
+	// span's own end.
+	// gitleaks:allow -- synthetic redaction fixture below
+	value := "sk-ant-" + strings.Repeat("sk-ant-", (64<<10)/len("sk-ant-"))
+	if got := RedactString(value, Options{}); got != RedactedSecret {
+		t.Fatalf("SETUP INVALID: the lookalike span was not redacted whole: got %q", got)
+	}
+	// Sixty-four candidates, each re-checked against the span, cost a small
+	// constant multiple of the span. A walk over every occurrence quadruples
+	// the cost when the span doubles, and the split path measured that walk
+	// at seventeen gigabytes on a one-megabyte chain.
+	limit := int64(200 * len(value))
+	if used := redactAllocBytes(value); used > limit {
+		t.Fatalf("redacting a %d byte lookalike span allocated %d bytes, over the %d byte limit", len(value), used, limit)
+	}
+}

internal/securefile/securefile.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"sync/atomic"
 	"time"
 )
@@ -30,13 +31,35 @@ 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
+	secretPath     string
+	publicationDir string
 }
 
 // NewCrypter returns a Crypter whose key lives at secretPath (created on first
@@ -45,11 +68,20 @@ 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, create)
+	secret, err := loadOrCreateSecret(c.secretPath, c.publicationDir, create)
 	if err != nil {
 		return nil, err
 	}
@@ -95,7 +127,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 string, create bool) ([]byte, error) {
+func loadOrCreateSecret(path, publicationDir string, create bool) ([]byte, error) {
 	if data, err := readSecretFileRetry(path); err == nil {
 		return data, nil
 	} else if !errors.Is(err, os.ErrNotExist) {
@@ -107,10 +139,10 @@ func loadOrCreateSecret(path string, create bool) ([]byte, error) {
 	if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil {
 		return nil, err
 	}
-	return createSecretFile(path)
+	return createSecretFile(path, publicationDir)
 }
 
-func createSecretFile(path string) ([]byte, error) {
+func createSecretFile(path, publicationDir string) ([]byte, error) {
 	lockPath := path + ".lock"
 	token := fmt.Sprintf("%d-%d-%d", os.Getpid(), time.Now().UnixNano(), secureLockSeq.Add(1))
 	var lastErr error
@@ -136,13 +168,13 @@ func createSecretFile(path string) ([]byte, error) {
 			} else if !errors.Is(rerr, os.ErrNotExist) {
 				return nil, rerr
 			}
-			return writeNewSecretFile(path)
+			return writeNewSecretFile(path, publicationDir)
 		}
 		// 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 oauth's createSecretFile --
-		// otherwise concurrent secret creation spuriously fails on Windows.
+		// as contention and retry, 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)
 		}
@@ -173,20 +205,63 @@ func reclaimStaleLock(lockPath, token string, staleAfter time.Duration) bool {
 		return false
 	}
 	if info, err := os.Stat(reclaimed); err == nil && time.Since(info.ModTime()) <= staleAfter {
-		_ = os.Rename(reclaimed, lockPath)
+		// 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)
 		return false
 	}
 	_ = os.Remove(reclaimed)
 	return true
 }
 
-func writeNewSecretFile(path string) ([]byte, error) {
+// 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) {
 	secret := make([]byte, secretBytes)
 	if _, err := io.ReadFull(rand.Reader, secret); err != nil {
 		return nil, fmt.Errorf("securefile: generate secret: %w", err)
 	}
-	dir := filepath.Dir(path)
-	tmp, err := os.CreateTemp(dir, filepath.Base(path)+".*.tmp")
+	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)
 	if err != nil {
 		return nil, fmt.Errorf("securefile: create secret temp file: %w", err)
 	}
@@ -200,7 +275,7 @@ func writeNewSecretFile(path string) ([]byte, error) {
 		_ = tmp.Close()
 		return nil, fmt.Errorf("securefile: write secret: %w", werr)
 	}
-	if serr := tmp.Sync(); serr != nil {
+	if serr := syncSecretFileFn(tmp); serr != nil {
 		_ = tmp.Close()
 		return nil, fmt.Errorf("securefile: sync secret: %w", serr)
 	}
@@ -210,6 +285,12 @@ func writeNewSecretFile(path 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,9 +1,12 @@
 package securefile
 
 import (
+	"bytes"
+	"errors"
 	"os"
 	"path/filepath"
 	"runtime"
+	"sync"
 	"testing"
 	"time"
 )
@@ -23,7 +26,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)
 	}
@@ -125,7 +128,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)
 	}
@@ -138,3 +141,198 @@ 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

@@ -0,0 +1,258 @@
+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

@@ -0,0 +1,351 @@
+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,9 +15,10 @@ 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.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
 	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 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.
+// 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.
 
 import (
 	"context"
@@ -61,16 +61,30 @@ 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
@@ -79,12 +93,14 @@ 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 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
+// 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
 // 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)
@@ -133,10 +149,11 @@ 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)
 	}
-	stagedManifest := filepath.Join(staged, skillFileName)
-	if err := os.WriteFile(stagedManifest, data, 0o644); err != nil {
-		return InstallResult{}, fmt.Errorf("stage SKILL.md: %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)
 	stagedData, err := os.ReadFile(stagedManifest)
 	if err != nil || hashContent(stagedData) != hash {
 		if err != nil {
@@ -169,8 +186,13 @@ 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}
+	lock[name] = LockEntry{Source: source, Hash: hash, TreeHash: treeHash}
 	if err := installtxn.CommitDir(target, staged, func() error {
 		return writeLock(dir, lock)
 	}); err != nil {
@@ -178,14 +200,26 @@ func Install(ctx context.Context, options InstallOptions) (InstallResult, error)
 	}
 
 	result := InstallResult{
-		Name:   name,
-		Path:   filepath.Join(target, skillFileName),
-		Hash:   hash,
-		Source: source,
+		Name:         name,
+		Path:         filepath.Join(target, skillFileName),
+		Hash:         hash,
+		TreeHash:     treeHash,
+		Source:       source,
+		SkippedLinks: skippedLinks,
 	}
 	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
 }
@@ -193,8 +227,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 hash can: the tree it
-// describes is the one the user was told they had.
+// shape cannot tell them apart and only the recorded hashes can: the tree they
+// describe 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)
@@ -215,24 +249,45 @@ func lockReconciler(dir string) installtxn.Reconciler {
 			// recorded one and recovery must not pick between them.
 			return installtxn.PhaseUnknown, nil
 		}
-		targetHash, err := manifestHash(target)
+		committed, err := treeMatchesEntry(target, entry)
 		if err != nil {
 			return installtxn.PhaseUnknown, err
 		}
-		if targetHash == entry.Hash {
+		if committed {
 			return installtxn.PhaseCommitted, nil
 		}
-		backupHash, err := manifestHash(backup)
+		prePublish, err := treeMatchesEntry(backup, entry)
 		if err != nil {
 			return installtxn.PhaseUnknown, err
 		}
-		if backupHash == entry.Hash {
+		if prePublish {
 			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
@@ -346,9 +401,10 @@ 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.HashDrift = skillHashDrift(skill, info.Hash, info.TreeHash)
 	return info, true
 }
 
@@ -556,11 +612,12 @@ func hashContent(data []byte) string {
 	return "sha256:" + hex.EncodeToString(sum[:])
 }
 
-// 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 {
+// 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 {
 	lockHash = strings.TrimSpace(lockHash)
 	if lockHash == "" {
 		return false
@@ -569,5 +626,13 @@ func skillHashDrift(skill Skill, lockHash string) bool {
 	if err != nil {
 		return true
 	}
-	return hashContent(data) != lockHash
+	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
 }

internal/skills/install_test.go

@@ -364,10 +364,11 @@ func TestInstallGitSourceUsesRunnerAndDoesNotExecuteContent(t *testing.T) {
 	if _, err := os.Stat(filepath.Join(cloneRoot, "PWNED")); err == nil {
 		t.Fatalf("install must never execute fetched content")
 	}
-	// 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")
+	// 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)
 	}
 }
 
@@ -440,10 +441,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")
 	}
 }
@@ -861,6 +862,146 @@ 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

@@ -0,0 +1,30 @@
+//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

@@ -0,0 +1,28 @@
+//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,8 +936,15 @@ func TestExecCommandInteractiveBlockSuggestsTTY(t *testing.T) {
 }
 
 func TestExecCommandTTYSkipsInteractiveBlock(t *testing.T) {
-	tool := NewScopedExecCommandTool(t.TempDir(), nil, newExecSessionManager())
-	// ssh to a bogus host fails fast on its own; the point is that the
+	// 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
 	// 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/grep.go

@@ -387,24 +387,24 @@ func scanGrepFile(ctx context.Context, resolvedRoot string, absolutePaths bool,
 
 	reader := bufio.NewReader(handle)
 	lineNumber := 1
-	sawLine := false
-	lastEnded := false
 	for {
 		if err := ctx.Err(); err != nil {
 			return err
 		}
 		raw, ended, err := readRawLine(reader)
+		// At EOF the scan stops where the line breaks stopped. A trailing
+		// line break is the boundary that CLOSED the line it ends — the rule
+		// the file tracker's countLines states for everything a reader
+		// reports, and an empty file has no lines at all. Emitting at EOF
+		// anyway handed every pattern that matches an empty string (anchors
+		// among them: ^$, \b, x*) a phantom match on a line that does not
+		// exist, numbered one past the real content.
 		if err == io.EOF {
-			if !sawLine || lastEnded {
-				return emitGrepLine(matcher, fileLabel, lineNumber, nil, emit)
-			}
 			return nil
 		}
 		if err != nil {
 			return nil
 		}
-		sawLine = true
-		lastEnded = ended
 		line := trimTrailingCarriageReturns(trimLineBreak(raw, ended))
 		if err := emitGrepLine(matcher, fileLabel, lineNumber, line, emit); err != nil {
 			return err

internal/tools/grep_line_end_test.go

@@ -0,0 +1,82 @@
+package tools
+
+import (
+	"context"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// A LINE BREAK ENDS THE LINE IT TRAILS; IT DOES NOT OPEN THE ONE AFTER IT.
+//
+// The scan used to hand the matcher an empty line past the file's real content
+// at EOF, so every pattern that matches an empty string — the anchored ones ^$
+// and \b among them — reported a match on a line no reader sees, numbered one
+// past the real content, and count mode charged that phantom to its total.
+// read_file and the file tracker's countLines both report "x\n" as one line
+// and an empty file as none; the grep scan now stops with them.
+func TestGrepStopsAtTheLineBreaksTheFileHas(t *testing.T) {
+	for name, testCase := range map[string]struct {
+		content string
+		pattern string
+		want    string
+	}{
+		"a trailing newline closes the line":          {"x\n", `^$`, "content:No matches found."},
+		"an unterminated last line stands alone":      {"x", `^$`, "content:No matches found."},
+		"an empty file has no lines":                  {"", `^$`, "content:No matches found."},
+		"the middle empty line is the only empty one": {"x\n\nz\n", `^$`, "content:a.txt:2: "},
+		"dot star does not run past the break":        {"x\n", ".*", "content:a.txt:1: x"},
+		"count stops at the trailing break":           {"x\n", `^$`, "count:0 matches found"},
+		"count charges only real lines":               {"x\n\nz\n", `^$`, "count:1 matches found"},
+		"count takes the trailing empty lines":        {"z\n\n\n", `^$`, "count:2 matches found"},
+		"an empty file counts nothing":                {"", `^$`, "count:0 matches found"},
+	} {
+		t.Run(name, func(t *testing.T) {
+			root := t.TempDir()
+			writeTestFile(t, filepath.Join(root, "a.txt"), testCase.content)
+
+			mode, want, found := strings.Cut(testCase.want, ":")
+			if !found {
+				t.Fatalf("SETUP INVALID: %s carries no mode", testCase.want)
+			}
+
+			result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{
+				"pattern":     testCase.pattern,
+				"output_mode": mode,
+			})
+			if result.Status != StatusOK {
+				t.Fatalf("grep status = %s: %s", result.Status, result.Output)
+			}
+			if result.Output != want {
+				t.Fatalf("grep = %q, want %q", result.Output, want)
+			}
+		})
+	}
+}
+
+// THE LINE NUMBERS THE SCAN REPORTS ARE THE LINE NUMBERS read_file USES, and
+// neither of them runs past the breaks the content carries. The pattern also
+// matches an empty line, so the line the scan used to invent at EOF would
+// appear right beside the real one: numbering a real line wrong in the same
+// file would fail the read side first, and the phantom would fail as a match
+// the content does not carry.
+func TestGrepLineNumbersAgreeWithReadFile(t *testing.T) {
+	root := t.TempDir()
+	writeTestFile(t, filepath.Join(root, "a.txt"), "one\nneedle two\nthree\n")
+
+	read := NewScopedReadFileTool(root, nil).Run(context.Background(), map[string]any{
+		"path": "a.txt",
+	})
+	if read.Status != StatusOK || !strings.Contains(read.Output, "2→needle two") {
+		t.Fatalf("SETUP INVALID: read_file does not number the needle line 2: %s", read.Output)
+	}
+	result := NewScopedGrepTool(root, nil).Run(context.Background(), map[string]any{
+		"pattern": "needle|^$",
+	})
+	if result.Status != StatusOK {
+		t.Fatalf("grep status = %s: %s", result.Status, result.Output)
+	}
+	if result.Output != "a.txt:2: needle two" {
+		t.Fatalf("grep = %q, want exactly the line read_file numbers 2; an extra empty match is a line the content does not have, so the scan opened one past the trailing break", result.Output)
+	}
+}

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(skill.Content)
+			return okResult(skills.ToolOutput(skill))
 		}
 		names = append(names, skill.Name)
 	}

internal/tools/skill_test.go

@@ -96,3 +96,27 @@ 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, Options{})
 	m.width = 120
 	m.height = 36
 	m.input.SetValue("/mcp")

internal/tui/hermetic_test.go

@@ -4,12 +4,100 @@ 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{})
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{
+	m := newViewTestModel(t, 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 := newModel(context.Background(), Options{
+	m := newViewTestModel(t, Options{
 		ProviderName: "custom-openai-compatible",
 		ModelName:    "MiniMax-M3",
 		ProviderProfile: config.ProviderProfile{

package-lock.json

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