fix(plugins): serialize empty sync-result arrays instead of null (#1817)

* fix(plugins): serialize empty sync-result arrays instead of null Sync's result slices (Added/Installed/Missing/Errors) stayed Go nil when nothing changed, and encoding/json marshals a nil slice as null. The frontend's SyncResult type declares these as non-nullable arrays and reads result.added.length unconditionally, so a routine "everything up to date" sync crashed with "can't access property 'length', e.added is null". Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(plugins): initialize sync-result slices at construction Per review feedback (Claude, Greptile): normalizing nil slices at each return site in sync() left the invariant fragile — a future early return would silently reintroduce the null-serialization bug. Initialize SyncResult's slices non-nil at the struct literal instead, and append the scan results onto them. Also tighten the regression test to check exact JSON field values instead of a raw strings.Contains("null") substring search. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Kandev Agent <agent@kandev.dev> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
2026-07-20 12:50:18 +01:00
parent ba75922fa1
commit 18ba644388
2 changed files with 48 additions and 4 deletions
@@ -58,22 +58,32 @@ func (s *Service) sync(ctx context.Context, includeTarballs bool) *SyncResult {
 	s.syncMu.Lock()
 	defer s.syncMu.Unlock()

-	result := &SyncResult{}
+	// Slices start non-nil: encoding/json marshals a nil slice as JSON null,
+	// but SyncResult's fields are typed as non-nullable arrays on the
+	// frontend (apps/web/lib/types/plugins.ts) — a null there crashes
+	// summarizeSyncResult's unconditional `.length` reads. append()-ing a
+	// possibly-nil result onto these keeps them non-nil.
+	result := &SyncResult{
+		Added:     []string{},
+		Installed: []string{},
+		Missing:   []string{},
+		Errors:    []SyncError{},
+	}
 	if s.pluginsDir == "" {
 		return result
 	}

 	added, sideloadErrs := s.scanDirSideloads()
-	result.Added = added
+	result.Added = append(result.Added, added...)
 	result.Errors = append(result.Errors, sideloadErrs...)

 	if includeTarballs {
 		installed, tarballErrs := s.scanTarballs(ctx)
-		result.Installed = installed
+		result.Installed = append(result.Installed, installed...)
 		result.Errors = append(result.Errors, tarballErrs...)
 	}

-	result.Missing = s.scanMissingInstalls()
+	result.Missing = append(result.Missing, s.scanMissingInstalls()...)

 	s.notifyDeliverer()
 	return result
@@ -2,6 +2,7 @@ package plugins

 import (
 	"context"
+	"encoding/json"
 	"os"
 	"path/filepath"
 	goruntime "runtime"
@@ -250,6 +251,39 @@ func TestServiceSync_MissingInstallAlreadyErrorIsIdempotent(t *testing.T) {
 	}
 }

+// TestServiceSync_EmptyResultSerializesEmptyArraysNotNull proves that a
+// no-op Sync (nothing added/installed/missing/errored) round-trips through
+// JSON as `[]`, not `null`. The frontend's SyncResult type
+// (apps/web/lib/types/plugins.ts) declares these fields as non-nullable
+// arrays and reads result.added.length unconditionally
+// (apps/web/lib/plugins/sync-summary.ts) — a `null` there throws
+// "can't access property 'length', e.added is null".
+func TestServiceSync_EmptyResultSerializesEmptyArraysNotNull(t *testing.T) {
+	svc, _, _ := newTestService(t)
+
+	result, err := svc.Sync(context.Background())
+	if err != nil {
+		t.Fatalf("Sync() unexpected error: %v", err)
+	}
+	if result.Added == nil || result.Installed == nil || result.Missing == nil || result.Errors == nil {
+		t.Fatalf("Sync() left a nil slice field: %+v", result)
+	}
+
+	data, err := json.Marshal(result)
+	if err != nil {
+		t.Fatalf("json.Marshal: %v", err)
+	}
+	var fields map[string]json.RawMessage
+	if err := json.Unmarshal(data, &fields); err != nil {
+		t.Fatalf("json.Unmarshal: %v", err)
+	}
+	for _, key := range []string{"added", "installed", "missing", "errors"} {
+		if raw, ok := fields[key]; !ok || string(raw) == "null" {
+			t.Fatalf("Sync() JSON field %q = %s, want an empty array: %s", key, raw, data)
+		}
+	}
+}
+
 // TestServiceSync_ConcurrentCallsAreSerialized proves Sync is guarded by a
 // mutex: a second concurrent Sync call must not proceed until the first one
 // has fully returned. Pauses the first call mid-Install (inside the fake