Plugins: a save that fails should say so, not vanish on redeploy

Reported symptom: every plugin comes back disabled after redeploying the
image, having been enabled before it. The persistence design was already
right - each compose file mounts api_data:/data and points PLUGINS_FILE at
/data/plugins.json - so the fault was that a failed write to that file was
invisible. Three defects, each confirmed with a test before being fixed:

A failed write was reported as success. Upsert set rec.Enabled before it
persisted, and the handler folded the resulting error into the same
200-with-warning used for "saved, but the connector failed to start". The
panel reloaded, read the in-memory record and showed the plugin enabled;
only a restart revealed that nothing had reached the disk. A save that
fails now rolls back in memory and returns 500, so the panel row shows the
error instead of "Saved".

A corrupt state file silently wiped the rest. Load returned an error,
main.go logged it and carried on with an empty record set, so the next
toggle overwrote plugins.json and took every other plugin's config with
it. An unreadable file is now moved aside to plugins.json.corrupt, and
persistLocked writes through a temp file + rename so an interrupted write
cannot produce that corrupt file in the first place.

A state file holding "null" panicked the server with "assignment to entry
in nil map" on the next save, and a null entry nil-dereferenced in Load.
Both now decode to "nothing configured".

Two changes make the next such failure loud rather than silent.
StartPlugins probes writability at boot and warns that plugin changes will
not survive a restart. And the API Server image gains the root entrypoint
the AIO image already had - chown /data, then drop to app via su-exec -
because a host bind mount (API_DATA=/srv/...) or a volume created before
/data existed arrives root-owned, and the unprivileged process cannot
write to it.

Not addressed here: a deployment that never reuses the named volume
(docker compose down -v, a renamed compose project, an anonymous volume
from a bare docker run) loses the file whatever the code does. The new
boot warning tells the two apart - writable but empty means the volume is
the problem, not permissions.

go build, go vet and go test ./... all pass. The Dockerfile change is
reviewed but not built: there is no Docker CLI on this machine, so the
su-exec privilege drop follows standard Alpine practice rather than an
observed run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

API Server/internal/api/plugins.go

@@ -46,16 +46,20 @@ func (s *Server) handleUpdatePlugin(w http.ResponseWriter, r *http.Request) {
 	}
 
 	v, err := s.plugins.Upsert(r.Context(), name, enabled, body.Config)
-	if err != nil {
-		if plugins.IsUnknown(err) {
-			writeError(w, http.StatusNotFound, "unknown plugin")
-			return
-		}
+	switch {
+	case err == nil:
+		writeJSON(w, http.StatusOK, map[string]any{"plugin": v})
+	case plugins.IsUnknown(err):
+		writeError(w, http.StatusNotFound, "unknown plugin")
+	case plugins.IsPersist(err):
+		// The change never reached plugins.json and has been rolled back.
+		// Reporting this as a 200-with-warning is what let a plugin look
+		// enabled in the panel and come back disabled after a redeploy.
+		writeError(w, http.StatusInternalServerError, err.Error())
+	default:
 		// A failed init (e.g. bad credentials) is reported but the state was saved.
 		writeJSON(w, http.StatusOK, map[string]any{"plugin": v, "warning": err.Error()})
-		return
 	}
-	writeJSON(w, http.StatusOK, map[string]any{"plugin": v})
 }
 
 // POST /api/admin/plugins — register an external (remote HTTP) plugin. Body:
@@ -76,6 +80,10 @@ func (s *Server) handleRegisterPlugin(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 	if err := s.plugins.RegisterExternal(body.Name, body.BaseURL, body.Provider); err != nil {
+		if plugins.IsPersist(err) {
+			writeError(w, http.StatusInternalServerError, err.Error())
+			return
+		}
 		writeError(w, http.StatusConflict, err.Error())
 		return
 	}
@@ -87,6 +95,10 @@ func (s *Server) handleRegisterPlugin(w http.ResponseWriter, r *http.Request) {
 // only be disabled).
 func (s *Server) handleDeletePlugin(w http.ResponseWriter, r *http.Request) {
 	if err := s.plugins.Remove(r.Context(), r.PathValue("name")); err != nil {
+		if plugins.IsPersist(err) {
+			writeError(w, http.StatusInternalServerError, err.Error())
+			return
+		}
 		writeError(w, http.StatusBadRequest, err.Error())
 		return
 	}

API Server/internal/api/server.go

@@ -185,6 +185,13 @@ func New(cfg config.Config, client *pb.Client) *Server {
 // non-blocking; if PocketBase is not yet configured it no-ops and the lazy path
 // rebuilds on first connect.
 func (s *Server) StartPlugins() error {
+	// Surface an unwritable state directory at boot. Without this the first
+	// symptom is a superadmin enabling plugins, seeing them work, and finding
+	// them all disabled after the next redeploy — because every save failed.
+	if err := s.plugins.CheckWritable(); err != nil {
+		log.Printf("WARNING: %v", err)
+		log.Printf("WARNING: plugin changes will NOT survive a restart — make the directory holding PLUGINS_FILE writable by the container user")
+	}
 	err := s.plugins.Load()
 	go func() {
 		ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)