fix(plugin,release,ci): CLI targets for non-cli plugins, catalog cache pairing, bump fixture TMPDIR, golden-path step 10 budget (D-0669, D-0875, D-0831, D-0307)

internal/plugin/catalog/cache.go

@@ -8,7 +8,9 @@ package catalog
 // catalog.etag beside it, both mode 0600.
 // Constraints: the ETag file is removed before the body is replaced and
 // written after it, so a crash between the two leaves a body with no ETag
-// (an unconditional refetch), never a new body paired with an old ETag.
+// (an unconditional refetch), never a new body paired with an old ETag. Both
+// files are read and written under one lock (cache_lock.go), so concurrent
+// processes cannot pair one response's body with another's ETag (D-0875).
 // Writes are best effort: a read-only cache dir never fails a command.
 
 import (
@@ -55,6 +57,14 @@ func readCache(dir string) (*cached, *Catalog, bool) {
 	if dir == "" {
 		return nil, nil, false
 	}
+	// Without the lock the body can still be read (it is replaced atomically)
+	// but the ETag cannot be trusted to belong to it, so it is dropped: the
+	// next fetch is unconditional, never a 304 against the wrong body (D-0875).
+	locked := false
+	if unlock, err := lockCache(dir); err == nil {
+		locked = true
+		defer unlock()
+	}
 	path := filepath.Join(dir, bodyFile)
 	info, err := os.Stat(path)
 	if err != nil || !info.Mode().IsRegular() || info.Size() > maxBody {
@@ -68,7 +78,10 @@ func readCache(dir string) (*cached, *Catalog, bool) {
 	if err != nil {
 		return nil, nil, false
 	}
-	etag, _ := os.ReadFile(filepath.Join(dir, etagFile))
+	var etag []byte
+	if locked {
+		etag, _ = os.ReadFile(filepath.Join(dir, etagFile))
+	}
 	return &cached{body: body, etag: strings.TrimSpace(string(etag)), at: info.ModTime()}, cat, true
 }
 
@@ -77,6 +90,13 @@ func writeCache(dir string, body []byte, etag string) {
 	if dir == "" || os.MkdirAll(dir, 0o755) != nil {
 		return
 	}
+	// No lock, no write: an unserialised write is what pairs a body with the
+	// wrong ETag, and the cache is optional.
+	unlock, err := lockCache(dir)
+	if err != nil {
+		return
+	}
+	defer unlock()
 	if err := os.Remove(filepath.Join(dir, etagFile)); err != nil && !errors.Is(err, fs.ErrNotExist) {
 		return
 	}

internal/plugin/catalog/cache_lock.go

@@ -0,0 +1,43 @@
+package catalog
+
+// cache_lock.go — serialise access to the on-disk catalog pair.
+//
+// Purpose: the body and the ETag are two files, so two writers (or a writer and
+// a reader) could interleave and pair one response's body with another's ETag;
+// a later 304 would then keep the wrong body (D-0875). Every read and write of
+// the pair holds one lock.
+// Inputs: the cache directory. Outputs: an unlock func.
+// Constraints: a process-wide mutex covers goroutines of one process; an flock
+// on catalog.lock beside the files covers other processes (cache_lock_unix.go;
+// a no-op without flock, where the mutex still holds). The lock is never held
+// across a network call.
+
+import (
+	"os"
+	"path/filepath"
+	"sync"
+)
+
+const lockFile = "catalog.lock"
+
+var cacheMu sync.Mutex
+
+// lockCache takes the cache lock for dir. The returned func releases it. An
+// error means the cross-process lock could not be taken; nothing is held then.
+func lockCache(dir string) (func(), error) {
+	cacheMu.Lock()
+	f, err := os.OpenFile(filepath.Join(dir, lockFile), os.O_CREATE|os.O_RDWR, 0o600)
+	if err != nil {
+		cacheMu.Unlock()
+		return nil, err
+	}
+	if err := flockFile(f); err != nil {
+		_ = f.Close()
+		cacheMu.Unlock()
+		return nil, err
+	}
+	return func() {
+		_ = f.Close() // closing drops the flock
+		cacheMu.Unlock()
+	}, nil
+}

internal/plugin/catalog/cache_lock_other.go

@@ -0,0 +1,12 @@
+//go:build !(linux || darwin || freebsd || netbsd || openbsd || dragonfly)
+
+package catalog
+
+// Purpose: the catalog cache lock on platforms without flock (Windows).
+// Constraints: a no-op; the process-wide mutex in lockCache still serialises
+// every goroutine of one process. Windows runs the unix build under WSL2 (VMI).
+
+import "os"
+
+// flockFile is a no-op without flock.
+func flockFile(*os.File) error { return nil }

internal/plugin/catalog/cache_lock_unix.go

@@ -0,0 +1,38 @@
+//go:build linux || darwin || freebsd || netbsd || openbsd || dragonfly
+
+package catalog
+
+// Purpose: the cross-process flock of the catalog cache (unix build).
+// Constraints: syscall.Flock only. The kernel drops the lock when the last
+// descriptor closes, process death included. The pattern is
+// internal/invoke/audit_lock_unix.go.
+
+import (
+	"errors"
+	"os"
+	"syscall"
+	"time"
+)
+
+// lockWait bounds how long a cache user waits for another process's lock: a
+// holder stuck forever must not hang a plugin command.
+var lockWait = 5 * time.Second
+
+// flockFile takes an exclusive flock on f, waiting at most lockWait.
+func flockFile(f *os.File) error {
+	deadline := time.Now().Add(lockWait)
+	for {
+		err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX|syscall.LOCK_NB)
+		switch {
+		case err == nil:
+			return nil
+		case errors.Is(err, syscall.EINTR):
+			continue
+		case !errors.Is(err, syscall.EWOULDBLOCK):
+			return err
+		case time.Now().After(deadline):
+			return errors.New("the catalog cache stayed locked by another process")
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+}

internal/plugin/catalog/cache_lock_unix_test.go

@@ -0,0 +1,76 @@
+//go:build linux || darwin || freebsd || netbsd || openbsd || dragonfly
+
+package catalog
+
+import (
+	"os"
+	"path/filepath"
+	"syscall"
+	"testing"
+	"time"
+)
+
+// TestWriteCacheWaitsForOtherProcessLock holds the flock through a separate
+// descriptor, as another nself process would, and proves a cache write waits
+// for it instead of interleaving (D-0875).
+func TestWriteCacheWaitsForOtherProcessLock(t *testing.T) {
+	dir := t.TempDir()
+	f, err := os.OpenFile(filepath.Join(dir, lockFile), os.O_CREATE|os.O_RDWR, 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close() //nolint:errcheck // test descriptor
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
+		t.Fatal(err)
+	}
+	done := make(chan struct{})
+	go func() {
+		writeCache(dir, sampleCatalog(1), `"e1"`)
+		close(done)
+	}()
+	select {
+	case <-done:
+		t.Fatal("writeCache finished while another process held the cache lock")
+	case <-time.After(150 * time.Millisecond):
+	}
+	if _, err := os.Stat(filepath.Join(dir, bodyFile)); err == nil {
+		t.Fatal("body was written under another process's lock")
+	}
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_UN); err != nil {
+		t.Fatal(err)
+	}
+	select {
+	case <-done:
+	case <-time.After(5 * time.Second):
+		t.Fatal("writeCache did not resume after the lock was released")
+	}
+	if c, _, ok := readCache(dir); !ok || c.etag != `"e1"` {
+		t.Fatalf("cache after release: ok=%v etag=%q", ok, c.etag)
+	}
+}
+
+// TestReadCacheDropsETagWhenLockTimesOut holds the flock through a separate
+// descriptor past lockWait: the read must not return an ETag it could not read
+// under the lock, or a later 304 would keep the wrong body (D-0875).
+func TestReadCacheDropsETagWhenLockTimesOut(t *testing.T) {
+	dir := t.TempDir()
+	writeCache(dir, sampleCatalog(1), `"e1"`)
+	f, err := os.OpenFile(filepath.Join(dir, lockFile), os.O_CREATE|os.O_RDWR, 0o600)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer f.Close() //nolint:errcheck // test descriptor
+	if err := syscall.Flock(int(f.Fd()), syscall.LOCK_EX); err != nil {
+		t.Fatal(err)
+	}
+	old := lockWait
+	lockWait = 50 * time.Millisecond
+	defer func() { lockWait = old }()
+	c, _, ok := readCache(dir)
+	if !ok {
+		t.Fatal("body should still be readable while the lock is held elsewhere")
+	}
+	if c.etag != "" {
+		t.Fatalf("etag %q returned without the cache lock", c.etag)
+	}
+}

internal/plugin/catalog/client_test.go

@@ -3,12 +3,14 @@ package catalog
 import (
 	"context"
 	"errors"
+	"fmt"
 	"net/http"
 	"net/http/httptest"
 	"os"
 	"path/filepath"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -192,3 +194,41 @@ func TestCommandHintReadsCacheOnly(t *testing.T) {
 		t.Fatal("unknown command offered as a hint")
 	}
 }
+
+// TestWriteCacheConcurrentKeepsPair proves racing writers and readers never see
+// one response's body with another's ETag (D-0875). Without the cache lock a
+// later 304 would keep the wrong body.
+func TestWriteCacheConcurrentKeepsPair(t *testing.T) {
+	dir := t.TempDir()
+	const writers, rounds = 8, 60
+	pairOK := func(c *cached) bool {
+		if c.etag == "" {
+			return true // a body with no ETag is a safe crash window
+		}
+		for n := 0; n < writers; n++ {
+			if c.etag == fmt.Sprintf(`"e%d"`, n) {
+				return strings.Contains(string(c.body), fmt.Sprintf("Reverse proxy %d", n))
+			}
+		}
+		return false
+	}
+	var wg sync.WaitGroup
+	var bad atomic.Int32
+	for w := 0; w < writers; w++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			for i := 0; i < rounds; i++ {
+				writeCache(dir, sampleCatalog(w), fmt.Sprintf(`"e%d"`, w))
+				if c, _, ok := readCache(dir); ok && !pairOK(c) {
+					bad.Add(1)
+				}
+			}
+		}()
+	}
+	wg.Wait()
+	c, _, ok := readCache(dir)
+	if !ok || !pairOK(c) || bad.Load() != 0 {
+		t.Fatalf("body and ETag diverged: final ok=%v pair=%v, %d bad reads", ok, ok && pairOK(c), bad.Load())
+	}
+}

internal/plugin/manifestv2/cli_targets.go

@@ -1,6 +1,7 @@
 package manifestv2
 
 import (
+	"encoding/json"
 	"strings"
 
 	"github.com/nself-org/cli/internal/errs"
@@ -88,13 +89,28 @@ func CheckCLITargets(plugin string, targets []CLITarget) error {
 	return nil
 }
 
-// CLITargetsOf is CLITargets over a parsed manifest through its compatibility
-// projection, which is what a released CLI reads from a v2 plugin.json.
-func CLITargetsOf(m *Manifest) []CLITarget {
-	p := Projection(m)
-	names := make([]string, 0, len(p.CLICommands))
-	for _, c := range p.CLICommands {
+// CLITargetsOf is CLITargets over the compatibility keys of a plugin.json, the
+// ones a released CLI's installer reads: name, pluginType, binaryName and
+// cliCommands. A v1 file carries them as written and a v2 file carries them
+// derived (Projection), so both give what the installer publishes.
+//
+// It reads the file rather than a parsed Manifest because normalizing a v1 file
+// drops a non-cli pluginType, and a service plugin that lists cliCommands has no
+// binary on the installer's v1 path while a file with cliCommands and no
+// pluginType has one per command (D-0669).
+func CLITargetsOf(data []byte) ([]CLITarget, error) {
+	var v struct {
+		Name        string       `json:"name"`
+		PluginType  string       `json:"pluginType"`
+		BinaryName  string       `json:"binaryName"`
+		CLICommands []CLICommand `json:"cliCommands"`
+	}
+	if err := json.Unmarshal(data, &v); err != nil {
+		return nil, invalid("plugin.json", err.Error())
+	}
+	names := make([]string, 0, len(v.CLICommands))
+	for _, c := range v.CLICommands {
 		names = append(names, c.Name)
 	}
-	return CLITargets(m.Name, p.PluginType, p.BinaryName, names)
+	return CLITargets(v.Name, v.PluginType, v.BinaryName, names), nil
 }

internal/plugin/manifestv2/cli_targets_test.go

@@ -25,3 +25,35 @@ func TestCLITargets(t *testing.T) {
 		}
 	}
 }
+
+// TestCLITargetsOf proves the targets match the installer's v1 rule for the
+// plugin.json as written (D-0669): a service plugin that lists cliCommands has
+// none, a plugin with cliCommands and no pluginType has one per command, and
+// the v2 form of each gives the same.
+func TestCLITargetsOf(t *testing.T) {
+	const meta = `"version":"1.0.0","description":"d","category":"infrastructure"`
+	cases := []struct {
+		name, doc string
+		want      int
+	}{
+		{"service with cliCommands", `{"name":"retro-gaming",` + meta + `,"pluginType":"service","cliCommands":[{"name":"server","description":"d"}]}`, 0},
+		{"cliCommands, no pluginType", `{"name":"tenant",` + meta + `,"cliCommands":[{"name":"tenant","description":"d"},{"name":"billing","description":"d"}]}`, 2},
+		{"cli type", `{"name":"tenant",` + meta + `,"pluginType":"cli","binaryName":"nself-tenant","cliCommands":[{"name":"tenant","description":"d"},{"name":"billing","description":"d"}]}`, 2},
+	}
+	for _, c := range cases {
+		got, err := CLITargetsOf([]byte(c.doc))
+		if err != nil || len(got) != c.want {
+			t.Errorf("%s: got %v, %v want %d targets", c.name, got, err, c.want)
+		}
+		if c.want == 0 {
+			continue // a v2 file has no pluginType to say the plugin is not cli
+		}
+		if m, err := ParseQuiet([]byte(c.doc)); err != nil {
+			t.Errorf("%s: parse: %v", c.name, err)
+		} else if b, err := Marshal(m); err != nil {
+			t.Errorf("%s: marshal: %v", c.name, err)
+		} else if got, err := CLITargetsOf(b); err != nil || len(got) != c.want {
+			t.Errorf("%s: v2 form: got %v, %v want %d targets", c.name, got, err, c.want)
+		}
+	}
+}

scripts/golden-path.sh

@@ -82,7 +82,16 @@ declare -A STEP_BASELINE=(
   [7]=15    # doctor --quick
   [8]=5     # license set (must precede the licensed plugin installs below)
   [9]=30    # plugin install ai
-  [10]=30   # plugin install claw
+  # claw installs six licensed dependencies (notify, google, mux, cron, voice,
+  # browser) and then itself: seven downloads from ping.nself.org. Per download
+  # the latency is bimodal: ~1s when ping is warm (step 10 took 4-6s on
+  # 2026-10-10 and 2026-10-11) and ~20s when it is slow (step 9, a single
+  # download, took 23-26s). Slow mode makes step 10 take 108s, 110s and 128s
+  # (runs 38202476017, 38087589999, 39192880837), so the old baseline of 30
+  # (fail at 90s) failed on ping latency with every container healthy. 60 ->
+  # warn 90s, fail 180s: room for seven slow downloads, and the warn line is the
+  # signal that ping.nself.org is slow (D-0307).
+  [10]=60   # plugin install claw (+ 6 licensed dependencies)
   [11]=10   # admin start
   [12]=5    # curl health
   # Step 13 is not a bare probe: it runs `nself build` + `nself start`, which

scripts/release/fixtures/cases4.sh

@@ -13,6 +13,8 @@ NEWH=bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
 # mk_bump_env: a nself/{cli,admin} tree at 1.4.13 with the golden tests present and a stub go.
 mk_bump_env() {
   B=$(mktemp -d "$BASE/bump.XXXXXX"); C=$B/nself/cli; A=$B/nself/admin
+  # mk_env's TMPDIR was deleted with its env; bump-version.sh's `mktemp -d` needs a live one (GNU mktemp fails without).
+  mkdir -p "$BASE/tmp"; export TMPDIR=$BASE/tmp
   mkdir -p "$C/scripts" "$C/internal/version" "$C/sdk/go" "$C/sdk/ts" "$C/sdk/ts-sdk" "$C/sdk/py" "$C/.github" \
     "$C/internal/build" "$C/cmd/commands" "$A/src/lib" "$B/bin"
   cp "$ROOT/scripts/bump-version.sh" "$C/scripts/"
@@ -55,6 +57,7 @@ gold_all() { local g; for g in $GOLD_PATHS; do grep -q "$1" "$C/$g" || return 1;
 # 13. bump regenerates the goldens, then re-runs the tests plain -------------------------
 mk_bump_env
 run_bump 9.9.9
+check "bump runs with a TMPDIR that exists" test -d "$TMPDIR"
 check "bump exits 0" test "$RC" -eq 0
 check "bump updated version.go" grep -q '"9.9.9"' "$C/internal/version/version.go"
 check "bump regenerated all four goldens" gold_all "build-version $NEWH"