fix(plugins): prune superseded plugin versions after a confirmed start (#2921)

Bounds plugin disk use: installing never removed the version it replaced, so
~/.kandev/plugins/<id>/ accumulated a full copy per install forever (4.2GB on
one machine, 3.74GB of it superseded versions). A plugin now keeps its active
version plus one rollback target, pruned only once the new version is
confirmed running.

Co-authored-by: Kandev Agent <agent@kandev.dev>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

apps/backend/internal/plugins/service.go

@@ -75,6 +75,18 @@ type Service struct {
 	// insertion one atomic catalog mutation across different plugin IDs.
 	agentToolInstallMu sync.Mutex
 
+	// extractingMu guards extractingPaths and, crucially, is held across the
+	// pkgtar extraction that registers into it: a version directory must never
+	// be observable on disk before it is marked in flight, or a concurrent
+	// prune could delete it in that gap. See extractPackage.
+	extractingMu sync.Mutex
+	// extractingPaths counts, per version directory, the installs that have
+	// extracted it but have not yet finished. Install extracts before it can
+	// know the plugin id (and therefore before it can take that id's lifecycle
+	// lock), so this is what tells a prune running under the lock that a
+	// directory belongs to an install still waiting for it.
+	extractingPaths map[string]int
+
 	pluginsDir       string
 	store            store.Store
 	registry         *Registry

apps/backend/internal/plugins/service_install.go

@@ -45,10 +45,11 @@ const downloadTimeout = 60 * time.Second
 // survive) and restarts the previous version's process, so a failed upgrade
 // attempt never destroys a previously working install.
 func (s *Service) Install(ctx context.Context, r io.Reader) (*store.Record, error) {
-	result, err := pkgtar.Install(r, s.pluginsDir)
+	result, err := s.extractPackage(r)
 	if err != nil {
 		return nil, err
 	}
+	defer s.releaseExtraction(result.InstallPath)
 	if err := s.checkMinKandevVersion(result.Manifest.MinKandevVersion); err != nil {
 		_ = os.RemoveAll(result.InstallPath)
 		return nil, err
@@ -123,6 +124,14 @@ func (s *Service) Install(ctx context.Context, r io.Reader) (*store.Record, erro
 	catalogLocked = false
 
 	activateErr := s.activate(rec)
+	if activateErr == nil {
+		// Only now can the new version be confirmed running, which is the
+		// point at which the versions it superseded stop being rollback
+		// targets. Pruning any earlier would delete what a failed upgrade
+		// falls back to; pruneSupersededVersions re-checks that the process is
+		// actually up before deleting anything.
+		s.pruneSupersededVersions(rec.ID, rec.Version, previousVersion(oldRec, hadOldRec))
+	}
 	s.notifyDeliverer()
 	s.notifyAgentToolCatalogChanged()
 
@@ -133,6 +142,65 @@ func (s *Service) Install(ctx context.Context, r io.Reader) (*store.Record, erro
 	return installed, activateErr
 }
 
+// extractPackage runs pkgtar.Install and registers the extracted version
+// directory as an in-flight install, both under extractingMu so the directory
+// is never visible on disk without being marked. Install cannot take the
+// per-plugin lifecycle lock any earlier than this — the id is only known once
+// the package's manifest has been parsed — so without the mark, two
+// overlapping installs of the same id would both extract, and whichever
+// acquired the lock first would prune the other's fresh directory and leave it
+// activating an InstallPath that no longer exists.
+//
+// The mutex serializes extraction across every plugin, not just one id. That
+// is deliberate and cheap: an install is a rare operator- or poller-driven
+// action, and the alternative (marking after pkgtar returns) leaves exactly
+// the gap this exists to close.
+func (s *Service) extractPackage(r io.Reader) (*pkgtar.InstallResult, error) {
+	s.extractingMu.Lock()
+	defer s.extractingMu.Unlock()
+
+	result, err := pkgtar.Install(r, s.pluginsDir)
+	if err != nil {
+		return nil, err
+	}
+	if s.extractingPaths == nil {
+		s.extractingPaths = make(map[string]int)
+	}
+	s.extractingPaths[result.InstallPath]++
+	return result, nil
+}
+
+// releaseExtraction drops one in-flight mark for path, deferred by Install so
+// it runs however that install ends (rejected package, failed persist, or a
+// completed activation).
+func (s *Service) releaseExtraction(path string) {
+	s.extractingMu.Lock()
+	defer s.extractingMu.Unlock()
+	if s.extractingPaths[path] > 1 {
+		s.extractingPaths[path]--
+		return
+	}
+	delete(s.extractingPaths, path)
+}
+
+// extractionInFlight reports whether some install has extracted path and not
+// yet finished with it.
+func (s *Service) extractionInFlight(path string) bool {
+	s.extractingMu.Lock()
+	defer s.extractingMu.Unlock()
+	return s.extractingPaths[path] > 0
+}
+
+// previousVersion returns the version an in-place upgrade replaced, or "" when
+// this install had no existing record to replace (a first install, or one over
+// a plugin whose record was lost) and therefore knows of no rollback target.
+func previousVersion(oldRec *store.Record, hadOldRec bool) string {
+	if !hadOldRec || oldRec == nil {
+		return ""
+	}
+	return oldRec.Version
+}
+
 // DevKandevVersion is the version string an un-stamped local build carries
 // (cmd/kandev's `Version` default, mirrored by internal/system/updates'
 // devVersion). It sorts meaninglessly against real semver, and a developer

apps/backend/internal/plugins/service_lifecycle.go

@@ -305,7 +305,16 @@ func (s *Service) StartActivePlugins(ctx context.Context) {
 				s.notifyDeliverer()
 				s.notifyAgentToolCatalogChanged()
 			}
+			continue
 		}
+		// Boot is the only trigger that reaches an instance which already
+		// accumulated superseded versions but installs nothing new (a plugin
+		// already at its latest version, or one whose auto-update is off), so
+		// it prunes too — under the same confirmed-start precondition as
+		// Install, which is what keeps disabled plugins, sideloads registered
+		// disabled, and plugins that fail to spawn untouched. Unlike Install,
+		// this loop holds no lifecycle lock, so the prune takes one itself.
+		s.pruneUnderLifecycleLock(rec.ID, rec.Version, "")
 	}
 }
 

apps/backend/internal/plugins/service_prune.go

@@ -0,0 +1,185 @@
+package plugins
+
+import (
+	"os"
+	"path/filepath"
+	"strings"
+
+	"go.uber.org/zap"
+
+	"github.com/kandev/kandev/internal/plugins/manifest"
+)
+
+// pluginDataDirName is the plugin-owned writable directory the runtime
+// manager creates beside a plugin's version directories
+// (<pluginsDir>/<id>/data — see internal/plugins/runtime's pluginCommand). It
+// is durable state that survives every upgrade and is removed only by
+// Uninstall, so the prune below excludes it by name as well as by the
+// manifest check every retained candidate has to pass.
+const pluginDataDirName = "data"
+
+// pruneSupersededVersions removes every superseded version directory under
+// <pluginsDir>/<id>/, keeping activeVersion plus one rollback target.
+// Installing never used to remove the version it replaced, so a plugin's
+// directory grew by a full copy (~20MB, ~95MB for a package shipping all five
+// platform binaries) on every install — and the auto-update poller installs
+// with no user action at all, which is how a single plugin reached 46
+// superseded copies and one machine reached 3.74GB of them.
+//
+// Retention is deliberately fixed at "current + 1 previous" rather than
+// configurable: the second directory exists so a failed upgrade still has a
+// working process to fall back on (see rollbackFailedInstall), which is a
+// safety invariant rather than an operator preference, and any larger number
+// re-creates the unbounded growth this exists to stop.
+//
+// Nothing is deleted unless id's process is confirmed running, which is the
+// precondition that makes this safe: pruning before the new version starts
+// would delete the very directory a failed upgrade needs. The check lives here
+// rather than at each call site so a future caller cannot forget it — note
+// that activate returns nil without starting anything when no runtime is
+// wired, so "activate succeeded" alone is not the same guarantee.
+//
+// Every failure is logged and swallowed: a plugin that installed and started
+// correctly must never be reported as failed because cleanup could not delete
+// a directory. Callers hold id's lifecycle lock, so no concurrent
+// install/enable/uninstall for the same id can race the scan.
+//
+// rollbackVersion names the version the caller knows was previously in use;
+// when it is empty (or no longer on disk) the semver-greatest remaining
+// version is retained instead.
+func (s *Service) pruneSupersededVersions(id, activeVersion, rollbackVersion string) {
+	if s.pluginsDir == "" || id == "" || activeVersion == "" {
+		return
+	}
+	if s.runtime == nil || !s.runtime.Running(id) {
+		return
+	}
+
+	// Every read and delete below goes through one os.Root handle opened on
+	// the plugin directory, so a path validated by isInstalledVersionDir is
+	// removed within the same confined tree: a symlink or rename planted
+	// mid-prune cannot redirect the removal outside <pluginsDir>/<id>/.
+	root, err := os.OpenRoot(filepath.Join(s.pluginsDir, id))
+	if err != nil {
+		if !os.IsNotExist(err) {
+			s.log.Warn("plugins: could not open plugin directory for pruning",
+				zap.String("plugin_id", id), zap.Error(err))
+		}
+		return
+	}
+	defer func() { _ = root.Close() }()
+
+	versions, err := installedVersionDirs(root, id)
+	if err != nil {
+		s.log.Warn("plugins: could not scan installed versions for pruning",
+			zap.String("plugin_id", id), zap.Error(err))
+		return
+	}
+
+	keep := retainedVersions(versions, activeVersion, rollbackVersion)
+	for _, version := range versions {
+		if keep[version] {
+			continue
+		}
+		// A concurrent Install extracts before it can take id's lifecycle
+		// lock, so a version directory that exists here may belong to an
+		// install still waiting on that lock. Deleting it would strand that
+		// caller with an InstallPath pointing at nothing.
+		if s.extractionInFlight(filepath.Join(s.pluginsDir, id, version)) {
+			continue
+		}
+		if err := root.RemoveAll(version); err != nil {
+			s.log.Warn("plugins: could not remove superseded plugin version",
+				zap.String("plugin_id", id), zap.String("version", version), zap.Error(err))
+			continue
+		}
+		s.log.Info("plugins: removed superseded plugin version",
+			zap.String("plugin_id", id), zap.String("version", version))
+	}
+}
+
+// pruneUnderLifecycleLock is pruneSupersededVersions for a caller that does
+// not already hold id's lifecycle lock (the boot activation loop). Install
+// holds it across activate and the prune that follows, and the prune relies on
+// that exclusion, so the boot path takes the same lock rather than depending
+// on nothing else being able to run yet.
+func (s *Service) pruneUnderLifecycleLock(id, activeVersion, rollbackVersion string) {
+	lock := s.lifecycleLocks.lockFor(id)
+	lock.Lock()
+	defer lock.Unlock()
+	s.pruneSupersededVersions(id, activeVersion, rollbackVersion)
+}
+
+// retainedVersions returns the set of version directory names to keep:
+// active, plus one rollback target — rollback when the caller named a version
+// that is still on disk, otherwise the semver-greatest of the rest.
+func retainedVersions(versions []string, active, rollback string) map[string]bool {
+	keep := map[string]bool{active: true}
+	fallback := ""
+	for _, version := range versions {
+		if version == active {
+			continue
+		}
+		if version == rollback {
+			keep[version] = true
+			return keep
+		}
+		if fallback == "" || manifest.CompareVersions(version, fallback) > 0 {
+			fallback = version
+		}
+	}
+	if fallback != "" {
+		keep[fallback] = true
+	}
+	return keep
+}
+
+// installedVersionDirs lists the entries directly under root (a handle on
+// <pluginsDir>/<id>/) that are installed versions of id. The bar is
+// deliberately high, because everything it returns is a deletion candidate:
+// the entry must be a real directory (a symlink reports as a link here and is
+// skipped), must not be the plugin's data directory or any dot-prefixed name
+// (which covers pkgtar's in-flight ".tmp-*" staging directories and any other
+// hidden entry), and must hold a manifest.yaml naming exactly this id and this
+// directory as its version — which is what pkgtar.Install writes and what
+// nothing else on that path produces. The plugin's store record ("<id>.yml")
+// and config ("<id>.config.yml") live one level up, beside the plugin
+// directory rather than inside it, so the root handle cannot reach them.
+func installedVersionDirs(root *os.Root, id string) ([]string, error) {
+	dir, err := root.Open(".")
+	if err != nil {
+		return nil, err
+	}
+	defer func() { _ = dir.Close() }()
+
+	entries, err := dir.ReadDir(-1)
+	if err != nil {
+		return nil, err
+	}
+	var versions []string
+	for _, entry := range entries {
+		name := entry.Name()
+		if !entry.IsDir() || name == pluginDataDirName || strings.HasPrefix(name, ".") {
+			continue
+		}
+		if !isInstalledVersionDir(root, id, name) {
+			continue
+		}
+		versions = append(versions, name)
+	}
+	return versions, nil
+}
+
+// isInstalledVersionDir reports whether <version>/manifest.yaml under root
+// parses and declares exactly this id and version.
+func isInstalledVersionDir(root *os.Root, id, version string) bool {
+	data, err := root.ReadFile(filepath.Join(version, manifestFileName))
+	if err != nil {
+		return false
+	}
+	m, err := manifest.Parse(data)
+	if err != nil {
+		return false
+	}
+	return m.ID == id && m.Version == version
+}

apps/backend/internal/plugins/service_prune_test.go

@@ -0,0 +1,517 @@
+package plugins
+
+import (
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	goruntime "runtime"
+	"sort"
+	"sync"
+	"testing"
+	"time"
+
+	"github.com/kandev/kandev/internal/plugins/store"
+)
+
+const prunePluginID = "kandev-plugin-slack"
+
+// seedVersionDir writes a version directory shaped exactly like one
+// pkgtar.Install extracts (a manifest.yaml whose id/version match the
+// directory name, plus a payload file), so the prune scan sees a real
+// superseded install rather than an empty directory.
+func seedVersionDir(t *testing.T, pluginsDir, id, version string) string {
+	t.Helper()
+	dir := filepath.Join(pluginsDir, id, version)
+	if err := os.MkdirAll(filepath.Join(dir, "server"), 0o755); err != nil {
+		t.Fatalf("MkdirAll(%q): %v", dir, err)
+	}
+	platformKey := goruntime.GOOS + "-" + goruntime.GOARCH
+	manifestYAML := fmt.Sprintf(`
+id: %s
+api_version: 1
+version: %s
+display_name: Test Plugin
+runtime:
+  type: binary
+  executables:
+    %s: server/plugin
+`, id, version, platformKey)
+	if err := os.WriteFile(filepath.Join(dir, manifestFileName), []byte(manifestYAML), 0o644); err != nil {
+		t.Fatalf("WriteFile(manifest.yaml): %v", err)
+	}
+	if err := os.WriteFile(filepath.Join(dir, "server", "plugin"), []byte("#!/bin/sh\n"), 0o755); err != nil {
+		t.Fatalf("WriteFile(server/plugin): %v", err)
+	}
+	return dir
+}
+
+// versionDirsOnDisk lists every directory entry under <pluginsDir>/<id>/,
+// sorted, so a test can assert the exact surviving set (including entries a
+// prune must never touch, like "data").
+func versionDirsOnDisk(t *testing.T, pluginsDir, id string) []string {
+	t.Helper()
+	entries, err := os.ReadDir(filepath.Join(pluginsDir, id))
+	if err != nil {
+		t.Fatalf("ReadDir(%q): %v", filepath.Join(pluginsDir, id), err)
+	}
+	var names []string
+	for _, entry := range entries {
+		if entry.IsDir() {
+			names = append(names, entry.Name())
+		}
+	}
+	sort.Strings(names)
+	return names
+}
+
+func assertVersionDirs(t *testing.T, pluginsDir, id string, want ...string) {
+	t.Helper()
+	got := versionDirsOnDisk(t, pluginsDir, id)
+	sort.Strings(want)
+	if len(got) != len(want) {
+		t.Fatalf("version dirs on disk = %v, want %v", got, want)
+	}
+	for i := range got {
+		if got[i] != want[i] {
+			t.Fatalf("version dirs on disk = %v, want %v", got, want)
+		}
+	}
+}
+
+// TestServiceInstallPrunesSupersededVersions pins the retention rule: after a
+// successful install whose new version started cleanly, only the newly active
+// version and the immediately-previous one (the rollback target) survive.
+// Every install used to leave its predecessor behind forever — 46 copies of a
+// single plugin, 3.74GB of superseded versions on a real machine.
+func TestServiceInstallPrunesSupersededVersions(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err != nil {
+		t.Fatalf("Install(1.1.0): %v", err)
+	}
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.2.0", false)); err != nil {
+		t.Fatalf("Install(1.2.0): %v", err)
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "1.1.0", "1.2.0")
+}
+
+// TestServiceInstallPrunesAccumulatedVersions covers the already-broken
+// instance: many superseded versions predate the fix, and one successful
+// install must collapse them to the retained pair.
+func TestServiceInstallPrunesAccumulatedVersions(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0, active
+	for _, v := range []string{"0.1.0", "0.2.0", "0.9.9"} {
+		seedVersionDir(t, dir, prunePluginID, v)
+	}
+
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err != nil {
+		t.Fatalf("Install(1.1.0): %v", err)
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "1.0.0", "1.1.0")
+}
+
+// TestServiceInstallPruneKeepsRollbackTarget pins that the first upgrade
+// prunes nothing: the previous version is the rollback target a failed
+// restart falls back to.
+func TestServiceInstallPruneKeepsRollbackTarget(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err != nil {
+		t.Fatalf("Install(1.1.0): %v", err)
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "1.0.0", "1.1.0")
+}
+
+// TestServiceInstallPruneKeepsDataDirectory pins the hard constraint: the
+// plugin's writable data directory is durable state that survives every
+// upgrade and is removed only on uninstall.
+func TestServiceInstallPruneKeepsDataDirectory(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+
+	marker := filepath.Join(dir, prunePluginID, "data", "marker.txt")
+	if err := os.MkdirAll(filepath.Dir(marker), 0o755); err != nil {
+		t.Fatalf("MkdirAll(data): %v", err)
+	}
+	if err := os.WriteFile(marker, []byte("keep me"), 0o644); err != nil {
+		t.Fatalf("WriteFile(marker): %v", err)
+	}
+
+	for _, v := range []string{"1.1.0", "1.2.0"} {
+		if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, v, false)); err != nil {
+			t.Fatalf("Install(%s): %v", v, err)
+		}
+	}
+
+	if _, err := os.Stat(marker); err != nil {
+		t.Fatalf("plugin data dir marker was removed by the prune: %v", err)
+	}
+	assertVersionDirs(t, dir, prunePluginID, "1.1.0", "1.2.0", "data")
+}
+
+// TestServiceInstallPruneNeverRemovesDataDirWithManifest pins the data
+// directory's exclusion by name, independently of the manifest check that
+// also happens to cover it. The directory is plugin-writable, so its contents
+// are not something the host gets to reason about: a plugin that writes its
+// own manifest.yaml there must not be able to talk the prune into deleting
+// its durable state.
+func TestServiceInstallPruneNeverRemovesDataDirWithManifest(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	seedVersionDir(t, dir, prunePluginID, pluginDataDirName)
+	marker := filepath.Join(dir, prunePluginID, pluginDataDirName, "marker.txt")
+	if err := os.WriteFile(marker, []byte("keep me"), 0o644); err != nil {
+		t.Fatalf("WriteFile(marker): %v", err)
+	}
+
+	for _, v := range []string{"1.1.0", "1.2.0"} {
+		if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, v, false)); err != nil {
+			t.Fatalf("Install(%s): %v", v, err)
+		}
+	}
+
+	if _, err := os.Stat(marker); err != nil {
+		t.Fatalf("plugin data dir was removed by the prune: %v", err)
+	}
+}
+
+// TestServiceInstallPruneSkipsNonVersionDirectories pins that the prune only
+// removes directories that actually parse as an installed version: an
+// operator's stray directory, a directory whose manifest names a different
+// version, and pkgtar's in-flight ".tmp-*" staging directory are all left
+// alone.
+func TestServiceInstallPruneSkipsNonVersionDirectories(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+
+	pluginDir := filepath.Join(dir, prunePluginID)
+	if err := os.MkdirAll(filepath.Join(pluginDir, "notes"), 0o755); err != nil {
+		t.Fatalf("MkdirAll(notes): %v", err)
+	}
+	// pkgtar extracts into a ".tmp-*" dir under the plugin directory before
+	// renaming it into place, so a concurrent install has a fully populated
+	// staging dir here that the prune must not delete out from under it.
+	staging := seedVersionDir(t, dir, prunePluginID, "1.3.0")
+	if err := os.Rename(staging, filepath.Join(pluginDir, ".tmp-inflight")); err != nil {
+		t.Fatalf("Rename(.tmp-inflight): %v", err)
+	}
+	// A directory whose manifest declares a different version than its own
+	// name is not something the installer produced; leave it to the operator.
+	mismatched := seedVersionDir(t, dir, prunePluginID, "0.8.0")
+	if err := os.Rename(mismatched, filepath.Join(pluginDir, "misnamed")); err != nil {
+		t.Fatalf("Rename(misnamed): %v", err)
+	}
+
+	for _, v := range []string{"1.1.0", "1.2.0"} {
+		if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, v, false)); err != nil {
+			t.Fatalf("Install(%s): %v", v, err)
+		}
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "1.1.0", "1.2.0", "notes", ".tmp-inflight", "misnamed")
+}
+
+// TestServiceInstallSpawnFailurePrunesNothing pins that pruning happens only
+// after the new version is confirmed started. A failed spawn must leave every
+// other version on disk, because the operator's way out is the previous
+// version.
+func TestServiceInstallSpawnFailurePrunesNothing(t *testing.T) {
+	svc, dir, _, rt := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+
+	rt.setStartErr(prunePluginID, errors.New("spawn failed"))
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err == nil {
+		t.Fatal("Install() expected an error when the new version fails to spawn")
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "0.9.0", "1.0.0", "1.1.0")
+}
+
+// TestServiceInstallSaveFailurePrunesNothingAndRestartsPrevious pins the
+// other failure mode: rollbackFailedInstall removes only the just-extracted
+// version and restarts the previous one, and the prune never runs.
+func TestServiceInstallSaveFailurePrunesNothingAndRestartsPrevious(t *testing.T) {
+	dir := t.TempDir()
+	fsStore := store.NewFSStore(dir)
+	failing := &failingSaveStore{Store: fsStore}
+	svc := NewService(failing, NewRegistry(), nil, testLogger(t))
+	svc.SetPluginsDir(dir)
+	rt := newFakeRuntime()
+	svc.SetRuntime(rt)
+
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0, active + running
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+
+	failing.armFailNextSave()
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err == nil {
+		t.Fatal("Install() expected an error from the simulated Save failure")
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "0.9.0", "1.0.0")
+	if !rt.Running(prunePluginID) {
+		t.Fatal("the previous version's process was not restarted after the failed upgrade")
+	}
+}
+
+// TestServiceInstallPruneFailureDoesNotFailInstall pins that cleanup is
+// best-effort: a plugin that installed and started correctly is never
+// reported as failed because a superseded directory could not be removed.
+func TestServiceInstallPruneFailureDoesNotFailInstall(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+
+	requireUnwritableVersionDir(t, filepath.Join(dir, prunePluginID), "0.9.0")
+
+	rec, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false))
+	if err != nil {
+		t.Fatalf("Install() failed because the prune could not delete a directory: %v", err)
+	}
+	if rec.Status != StatusActive {
+		t.Fatalf("Install() Status = %q, want %q", rec.Status, StatusActive)
+	}
+}
+
+// requireUnwritableVersionDir makes pluginDir/version reject removal of its
+// contents, skipping the test when the current executor does not enforce the
+// permission bit (root, or a filesystem/OS that ignores it). The version is a
+// parameter rather than a constant so the coupling to what the caller seeded
+// is visible at the call site: a caller that seeded a different version would
+// otherwise get a silent no-op chmod.
+func requireUnwritableVersionDir(t *testing.T, pluginDir, version string) {
+	t.Helper()
+	probeParent := filepath.Join(t.TempDir(), "probe")
+	if err := os.MkdirAll(filepath.Join(probeParent, "child"), 0o755); err != nil {
+		t.Fatalf("MkdirAll(probe): %v", err)
+	}
+	if err := os.Chmod(probeParent, 0o555); err != nil {
+		t.Skipf("chmod unsupported on this platform: %v", err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(probeParent, 0o755) })
+	if err := os.RemoveAll(filepath.Join(probeParent, "child")); err == nil {
+		t.Skip("this executor ignores the directory write bit (running as root?)")
+	}
+
+	// The install itself must still be able to create the new version
+	// directory, so make only the superseded version's own contents
+	// undeletable rather than the plugin directory.
+	stale := filepath.Join(pluginDir, version)
+	if err := os.Chmod(stale, 0o555); err != nil {
+		t.Skipf("chmod unsupported on this platform: %v", err)
+	}
+	t.Cleanup(func() { _ = os.Chmod(stale, 0o755) })
+}
+
+// TestServiceInstallWithoutRuntimePrunesNothing pins the precondition against
+// the one path where activate reports success without starting anything: with
+// no runtime wired, activate only persists StatusActive, so there is no
+// confirmed start and no version may be discarded.
+func TestServiceInstallWithoutRuntimePrunesNothing(t *testing.T) {
+	dir := t.TempDir()
+	svc := NewService(store.NewFSStore(dir), NewRegistry(), nil, testLogger(t))
+	svc.SetPluginsDir(dir)
+
+	for _, v := range []string{"1.0.0", "1.1.0", "1.2.0"} {
+		if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, v, false)); err != nil {
+			t.Fatalf("Install(%s): %v", v, err)
+		}
+	}
+
+	assertVersionDirs(t, dir, prunePluginID, "1.0.0", "1.1.0", "1.2.0")
+}
+
+// TestPruneSkipsInFlightExtraction pins the guard against a concurrent
+// install's freshly extracted directory. Install extracts before it can know
+// the plugin id, so it cannot hold that id's lifecycle lock yet; a prune
+// running under the lock must still leave that directory alone, and must
+// reclaim it once the install that extracted it is done.
+func TestPruneSkipsInFlightExtraction(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0
+	if _, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false)); err != nil {
+		t.Fatalf("Install(1.1.0): %v", err)
+	}
+
+	// Exactly what a competing install has done by the time it blocks on the
+	// lifecycle lock: extracted and marked in flight, nothing else.
+	inFlight, err := svc.extractPackage(testPackage(t, prunePluginID, "1.2.0", false))
+	if err != nil {
+		t.Fatalf("extractPackage(1.2.0): %v", err)
+	}
+
+	svc.pruneSupersededVersions(prunePluginID, "1.1.0", "1.0.0")
+	assertVersionDirs(t, dir, prunePluginID, "1.0.0", "1.1.0", "1.2.0")
+
+	svc.releaseExtraction(inFlight.InstallPath)
+	svc.pruneSupersededVersions(prunePluginID, "1.1.0", "1.0.0")
+	assertVersionDirs(t, dir, prunePluginID, "1.0.0", "1.1.0")
+}
+
+// TestConcurrentInstallsKeepBothExtractions drives the interleaving end to
+// end: the first install is held inside its store write, holding the lifecycle
+// lock, while a second install for the same id extracts a different version
+// behind it. The first must not prune the second's package out from under it.
+func TestConcurrentInstallsKeepBothExtractions(t *testing.T) {
+	dir := t.TempDir()
+	barrier := &armedBarrierStore{Store: store.NewFSStore(dir)}
+	svc := NewService(barrier, NewRegistry(), nil, testLogger(t))
+	svc.SetPluginsDir(dir)
+	svc.SetRuntime(newFakeRuntime())
+
+	// 1.0.0 must be a real installed record, so the first racing install
+	// treats it as its rollback target and 1.2.0 becomes a prune candidate.
+	installTestPlugin(t, svc, prunePluginID)
+
+	entered, release := barrier.arm()
+	errs := make(chan error, 2)
+	go func() {
+		_, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.1.0", false))
+		errs <- err
+	}()
+	<-entered // 1.1.0 extracted, lifecycle lock held, prune still ahead
+
+	go func() {
+		_, err := svc.Install(context.Background(), testPackage(t, prunePluginID, "1.2.0", false))
+		errs <- err
+	}()
+	waitForVersionDir(t, dir, prunePluginID, "1.2.0") // extracted, now blocked on the lock
+
+	close(release)
+	if err := <-errs; err != nil {
+		t.Fatalf("concurrent install error: %v", err)
+	}
+	if err := <-errs; err != nil {
+		t.Fatalf("concurrent install error: %v", err)
+	}
+
+	for _, version := range []string{"1.1.0", "1.2.0"} {
+		if _, statErr := os.Stat(filepath.Join(dir, prunePluginID, version)); statErr != nil {
+			t.Fatalf("version %s was pruned out from under a concurrent install: %v", version, statErr)
+		}
+	}
+	rec, err := svc.Get(prunePluginID)
+	if err != nil {
+		t.Fatalf("Get(): %v", err)
+	}
+	if _, statErr := os.Stat(rec.InstallPath); statErr != nil {
+		t.Fatalf("the winning record's InstallPath %q does not exist: %v", rec.InstallPath, statErr)
+	}
+}
+
+// armedBarrierStore pauses one chosen Save, so a test can pin an install
+// inside the lifecycle-locked region of Install (its record write) while a
+// competing install runs behind it. Unarmed saves pass straight through, which
+// keeps ordinary setup installs out of the barrier.
+type armedBarrierStore struct {
+	store.Store
+	mu      sync.Mutex
+	armed   bool
+	entered chan struct{}
+	release chan struct{}
+}
+
+// arm makes the next Save block; entered closes once that Save is reached and
+// the returned release channel unblocks it when closed.
+func (s *armedBarrierStore) arm() (entered <-chan struct{}, release chan struct{}) {
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	s.entered = make(chan struct{})
+	s.release = make(chan struct{})
+	s.armed = true
+	return s.entered, s.release
+}
+
+func (s *armedBarrierStore) Save(rec *store.Record) error {
+	s.mu.Lock()
+	armed := s.armed
+	s.armed = false
+	entered, release := s.entered, s.release
+	s.mu.Unlock()
+	if armed {
+		close(entered)
+		<-release
+	}
+	return s.Store.Save(rec)
+}
+
+// waitForVersionDir blocks until a version directory appears, so a test can
+// synchronize on a competing install's extraction (a real side effect) rather
+// than on a sleep.
+func waitForVersionDir(t *testing.T, pluginsDir, id, version string) {
+	t.Helper()
+	dir := filepath.Join(pluginsDir, id, version)
+	deadline := time.NewTimer(10 * time.Second)
+	defer deadline.Stop()
+	tick := time.NewTicker(time.Millisecond)
+	defer tick.Stop()
+	for {
+		if _, err := os.Stat(dir); err == nil {
+			return
+		}
+		select {
+		case <-tick.C:
+		case <-deadline.C:
+			t.Fatalf("version dir %q never appeared", dir)
+		}
+	}
+}
+
+// TestStartActivePluginsPrunesAfterHealthyStart pins boot-time recovery: an
+// instance that already accumulated superseded versions collapses them on the
+// next restart, without waiting for an install that may never come.
+func TestStartActivePluginsPrunesAfterHealthyStart(t *testing.T) {
+	svc, dir, _, rt := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0, active
+	for _, v := range []string{"0.7.0", "0.8.0", "0.9.0"} {
+		seedVersionDir(t, dir, prunePluginID, v)
+	}
+	rt.Stop(prunePluginID) // simulate a fresh process: registered active, not running
+
+	svc.StartActivePlugins(context.Background())
+
+	if !rt.Running(prunePluginID) {
+		t.Fatal("StartActivePlugins() did not start the active plugin")
+	}
+	assertVersionDirs(t, dir, prunePluginID, "0.9.0", "1.0.0")
+}
+
+// TestStartActivePluginsDoesNotPruneWhenStartFails pins the same
+// confirmed-start precondition at boot.
+func TestStartActivePluginsDoesNotPruneWhenStartFails(t *testing.T) {
+	svc, dir, _, rt := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0, active
+	seedVersionDir(t, dir, prunePluginID, "0.8.0")
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+	rt.Stop(prunePluginID)
+	rt.setStartErr(prunePluginID, errors.New("spawn failed"))
+
+	svc.StartActivePlugins(context.Background())
+
+	assertVersionDirs(t, dir, prunePluginID, "0.8.0", "0.9.0", "1.0.0")
+}
+
+// TestStartActivePluginsDoesNotPruneDisabledPlugin pins that a plugin nobody
+// started keeps every version an operator staged for it.
+func TestStartActivePluginsDoesNotPruneDisabledPlugin(t *testing.T) {
+	svc, dir, _, _ := newTestServiceWithDir(t)
+	installTestPlugin(t, svc, prunePluginID) // 1.0.0, active
+	seedVersionDir(t, dir, prunePluginID, "0.8.0")
+	seedVersionDir(t, dir, prunePluginID, "0.9.0")
+	if err := svc.Disable(prunePluginID); err != nil {
+		t.Fatalf("Disable(): %v", err)
+	}
+
+	svc.StartActivePlugins(context.Background())
+
+	assertVersionDirs(t, dir, prunePluginID, "0.8.0", "0.9.0", "1.0.0")
+}

docs/decisions/2026-08-22-plugin-version-retention.md

@@ -0,0 +1,82 @@
+# ADR-2026-08-22-plugin-version-retention: Keep Exactly One Superseded Plugin Version
+
+**Status:** accepted
+**Date:** 2026-08-22
+**Area:** backend
+
+## Context
+
+Every plugin install extracts a full package into `~/.kandev/plugins/<id>/<version>/`, and no
+install path ever removed the version it replaced. Keeping the predecessor during the upgrade is
+deliberate: `Service.Install` stops the running plugin before swapping, and the old directory is
+what `rollbackFailedInstall` restarts when the new version fails. Nothing, however, removed it
+afterwards.
+
+All four install routes (marketplace/URL, auto-update poller, sideload upload, dropped tarball)
+converge on that same code, and the auto-update poller installs with no user action at all, so the
+growth is unattended. A version directory is roughly 20MB, or 95MB for a package shipping all five
+platform binaries. On one developer machine the plugins directory had reached 4.2GB, of which
+3.74GB was 58 superseded version directories, 46 of them belonging to a single auto-updating
+plugin.
+
+## Decision
+
+A plugin converges on two extracted versions: the active one and the version it replaced. The
+retained count is fixed in code, not configurable. It is a convergent state rather than an enforced
+cap, because every precondition below can legitimately leave more versions on disk.
+
+Deletion is gated on a **confirmed running process**, never on the install alone. `Install` prunes
+after `activate` returns successfully and boot (`StartActivePlugins`) prunes after `runtime.Start`
+succeeds, but in both cases `pruneSupersededVersions` itself re-checks `runtime.Running(id)` before
+deleting anything — `activate` returns nil without starting a process when no runtime is wired, so
+"activate succeeded" is not the same guarantee. A failed install, a failed spawn, a disabled plugin,
+a sideload registered disabled, and a plugin in `error` therefore all keep every version they have,
+as does a plugin whose deletion failed. `rollbackFailedInstall` is unchanged, and remains the one
+path that deletes a version directory without a prune: it removes the newly extracted version, and
+only that one, when the record cannot be persisted.
+
+Extraction is serialized and tracked. `Install` cannot take a plugin's lifecycle lock until it has
+parsed the manifest, so two overlapping installs of the same id both extract before either is
+serialized; the extraction and its in-flight registration happen under one mutex, and a prune skips
+any directory still marked in flight. Without that, the first install to acquire the lock would
+delete the second's fresh package and strand it activating a path that no longer exists.
+
+A directory is a deletion candidate only if it is a real directory (never a symlink), is not
+`data/` or any dot-prefixed name (which covers pkgtar's `.tmp-*` staging directories), and contains
+a `manifest.yaml` declaring exactly that plugin id and that directory name as its version. Listing,
+manifest reads, and removal all go through a single `os.Root` handle on the plugin directory, so a
+path validated by the scan is removed inside the same confined tree and the `<id>.yml` /
+`<id>.config.yml` records one level up are unreachable.
+
+Pruning is best-effort: every failure is logged and swallowed, because a plugin that installed and
+started correctly must not be reported as failed because cleanup could not delete a directory.
+
+## Consequences
+
+- Disk use per plugin becomes bounded by two versions instead of growing with every install.
+- Rollback keeps working: the version a failed upgrade restarts is always still on disk.
+- Boot-time pruning lets an instance that already accumulated versions recover on its next restart,
+  including plugins that are already at their latest version and would never trigger an install.
+- The prune deletes durable on-disk state, so its preconditions are the safety contract and any
+  change to them needs regression coverage (`service_prune_test.go`).
+- Rolling back further than one version now requires reinstalling the wanted package.
+
+## Alternatives Considered
+
+- **Make the retained count configurable.** Rejected as speculative. The second directory exists to
+  serve a specific mechanism (rollback of a failed upgrade), which is a safety invariant rather than
+  an operator preference, and any larger configured value re-creates the unbounded growth this
+  exists to stop. A knob would also need UI, docs, and validation for no demonstrated need.
+- **Keep only the active version.** Rejected because it deletes the rollback target
+  `rollbackFailedInstall` depends on, converting a failed upgrade from recoverable into an outage.
+- **Prune immediately after extraction, before the new version starts.** Rejected for the same
+  reason: it removes the fallback exactly when it is most likely to be needed.
+- **Age-based retention (delete versions older than N days).** Rejected because it neither bounds
+  disk use for a frequently-updating plugin nor guarantees a rollback target exists for a rarely
+  updated one.
+- **Install-time pruning only.** Rejected because the instances that motivated this fix are already
+  full of superseded versions, and a plugin at its latest version, or one with auto-update off,
+  would never install again and never recover.
+- **A separate operator-triggered cleanup action.** Rejected as a worse default: the growth is
+  unattended, so the recovery should be too, and an operator who has not noticed 3.74GB of stale
+  packages will not press a button about them.

docs/decisions/INDEX.md

@@ -187,3 +187,4 @@ Read individual ADRs for full context. Create new ones via `/record decision` or
 | 2026-08-19-repository-qualified-comparison-targets | [Qualify Git Comparison Targets by Repository](2026-08-19-repository-qualified-comparison-targets.md) | accepted | backend, agentctl, frontend, protocol, GitHub, GitLab | 2026-08-19 |
 | 2026-08-20-startup-configuration-source-parity | [Startup Configuration Uses One Typed Source Model](2026-08-20-startup-configuration-source-parity.md) | accepted | backend, agentctl, frontend, cli, security, operations | 2026-08-20 |
 | 2026-08-20-acp-client-non-underscore-extension-methods | [Route Non-Underscore Inbound Client Methods to the Extension Handler](2026-08-20-acp-client-non-underscore-extension-methods.md) | accepted | backend, protocol | 2026-08-20 |
+| 2026-08-22-plugin-version-retention | [Keep Exactly One Superseded Plugin Version](2026-08-22-plugin-version-retention.md) | accepted | backend | 2026-08-22 |

docs/public/plugins-authoring.md

@@ -44,7 +44,12 @@ flowchart LR
 
 Installation validates the manifest, archive paths, checksums, managed runtime,
 and the current host executable before extraction. Kandev then supervises the
-declared subprocess and injects a Host connection. The UI bundle is static
+declared subprocess and injects a Host connection. Installing a new version
+does not remove the one it replaces: that version stays on disk as the rollback
+target, and only the versions before it are deleted, once the new one is
+confirmed running. Two extracted versions is therefore the steady state for a
+plugin that runs; a plugin that is disabled or failed to start keeps every
+version it has, and `data/` is never part of that cleanup. The UI bundle is static
 package content loaded by the browser; it does not run inside the backend
 subprocess. On disable or uninstall, the host calls destroy when present and
 bulk-revokes registrations, styles, routes, handlers, and navigation. Reloads and

docs/public/plugins.md

@@ -127,6 +127,14 @@ Either path runs the same pipeline:
    Before persistence, credential-like values such as PATs, bearer tokens,
    labeled secrets/API keys, and the host home path are redacted; plugin stdout
    is not stored verbatim.
+6. Once the new version is confirmed running, delete the plugin's older
+   extracted versions, keeping the version now running plus the one it
+   replaced as a rollback target (see "Version retention on disk"). This step
+   is skipped entirely when the install or the spawn fails, and a deletion that
+   fails is logged without failing the install. Skipping it is not the same as
+   deleting nothing: an install that extracted successfully but could not be
+   recorded is rolled back, which removes that one new version directory and
+   restarts the previous one.
 
 A successful install that failed to spawn returns HTTP 201 with a
 `warning` field rather than failing outright. The package is installed,
@@ -247,6 +255,25 @@ host-managed runtime fields, including `signed`, `last_error`, and
 `last_error_at`. The diagnostic fields are empty until a runtime failure is
 recorded and are cleared after successful recovery.
 
+### Version retention on disk
+
+Two extracted versions is the steady state for a plugin that runs: the one
+currently running and the one it replaced, which is what a failed upgrade falls
+back to. Older versions are deleted once a newer one is confirmed running,
+either right after the install that superseded them or at the next backend
+start. The retained count is fixed and not configurable.
+
+It is a steady state rather than a hard cap, because the cleanup is
+conservative and never insists. Kandev deletes a version directory only after
+confirming that the plugin's process is up, so a plugin that is disabled,
+errored, or was never started keeps every version it has, and a deletion that
+fails leaves that version in place until the next attempt. It never
+touches `data/`, a directory whose name does not match the version declared by
+the `manifest.yaml` inside it, or anything outside `~/.kandev/plugins/<id>/`.
+Because an upgrade previously left its predecessor behind forever, an instance
+that has been auto-updating for a long time can reclaim a substantial amount of
+disk on its first restart after upgrading to this version.
+
 ## Security posture
 
 - **Auth is the spawn relationship.** Kandev spawns the plugin subprocess