From 18ba644388ba0b33cfdb68094d8f36befdbf3746 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Mon, 20 Jul 2026 12:50:18 +0100 Subject: [PATCH] fix(plugins): serialize empty sync-result arrays instead of null (#1817) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * 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 --------- Co-authored-by: Kandev Agent Co-authored-by: Claude Sonnet 5 --- apps/backend/internal/plugins/service_sync.go | 18 +++++++--- .../internal/plugins/service_sync_test.go | 34 +++++++++++++++++++ 2 files changed, 48 insertions(+), 4 deletions(-) diff --git a/apps/backend/internal/plugins/service_sync.go b/apps/backend/internal/plugins/service_sync.go index 72af28dc32..42ac064f29 100644 --- a/apps/backend/internal/plugins/service_sync.go +++ b/apps/backend/internal/plugins/service_sync.go @@ -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 diff --git a/apps/backend/internal/plugins/service_sync_test.go b/apps/backend/internal/plugins/service_sync_test.go index b83acda53c..8086c379f7 100644 --- a/apps/backend/internal/plugins/service_sync_test.go +++ b/apps/backend/internal/plugins/service_sync_test.go @@ -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