fix: address code review issues for pluggable widget engine

- Use atomic.Uint32 for placeholderCounter to prevent data races
- Only ignore os.IsNotExist in loadDefinitionsFromDir, warn on other errors
- Propagate registry init failure reason in unsupported widget type errors
- Document LSP completion cache sync.Once limitation

Author: engalar

cmd/mxcli/lsp_completion.go

@@ -79,6 +79,8 @@ func mdlCompletionItems(linePrefixUpper string) []protocol.CompletionItem {
 }
 
 // widgetRegistryCompletions returns completion items for registered widget types.
+// NOTE: Cached via sync.Once — new .def.json files added while the LSP server is
+// running will not appear until the server is restarted.
 var (
 	widgetCompletionsOnce sync.Once
 	widgetCompletionItems []protocol.CompletionItem

mdl/executor/cmd_pages_builder.go

@@ -34,8 +34,9 @@ type pageBuilder struct {
 	themeRegistry    *ThemeRegistry                     // Theme design property definitions (may be nil)
 
 	// Pluggable widget engine (lazily initialized)
-	widgetRegistry  *WidgetRegistry
-	pluggableEngine *PluggableWidgetEngine
+	widgetRegistry      *WidgetRegistry
+	pluggableEngine     *PluggableWidgetEngine
+	pluggableEngineErr  error // stores init failure reason for better error messages
 
 	// Per-operation caches (may change during execution)
 	layoutsCache    []*pages.Layout
@@ -54,7 +55,8 @@ func (pb *pageBuilder) initPluggableEngine() {
 	}
 	registry, err := NewWidgetRegistry()
 	if err != nil {
-		fmt.Fprintf(os.Stderr, "warning: pluggable widget registry init failed: %v\n", err)
+		pb.pluggableEngineErr = fmt.Errorf("widget registry init failed: %w", err)
+		fmt.Fprintf(os.Stderr, "warning: %v\n", pb.pluggableEngineErr)
 		return
 	}
 	if pb.reader != nil {

mdl/executor/cmd_pages_builder_v3.go

@@ -331,6 +331,9 @@ func (pb *pageBuilder) buildWidgetV3(w *ast.WidgetV3) (pages.Widget, error) {
 				return pb.pluggableEngine.Build(def, w)
 			}
 		}
+		if pb.pluggableEngineErr != nil {
+			return nil, fmt.Errorf("unsupported V3 widget type: %s (%v)", w.Type, pb.pluggableEngineErr)
+		}
 		return nil, fmt.Errorf("unsupported V3 widget type: %s", w.Type)
 	}
 

mdl/executor/widget_registry.go

@@ -109,7 +109,11 @@ func (r *WidgetRegistry) LoadUserDefinitions(projectPath string) error {
 func (r *WidgetRegistry) loadDefinitionsFromDir(dir string) error {
 	entries, err := os.ReadDir(dir)
 	if err != nil {
-		return nil // directory doesn't exist or not readable — not an error
+		if os.IsNotExist(err) {
+			return nil
+		}
+		fmt.Fprintf(os.Stderr, "warning: cannot read widget definitions from %s: %v\n", dir, err)
+		return nil
 	}
 
 	for _, entry := range entries {

sdk/widgets/augment.go

@@ -5,6 +5,7 @@ package widgets
 import (
 	"encoding/json"
 	"fmt"
+	"sync/atomic"
 
 	"github.com/mendixlabs/mxcli/sdk/widgets/mpk"
 )
@@ -560,20 +561,20 @@ func buildNestedObjectType(children []mpk.PropertyDef) map[string]any {
 
 // --- Helpers ---
 
-// placeholderCounter generates sequential placeholder IDs.
-var placeholderCounter uint32
+// placeholderCounter generates sequential placeholder IDs (atomic for concurrent safety).
+var placeholderCounter atomic.Uint32
 
 // placeholderID generates a placeholder hex ID. These will be remapped by collectIDs
 // in GetTemplateFullBSON, so exact values don't matter — they just need to be unique
 // 32-char hex strings.
 func placeholderID() string {
-	placeholderCounter++
-	return fmt.Sprintf("aa000000000000000000000000%06x", placeholderCounter)
+	n := placeholderCounter.Add(1)
+	return fmt.Sprintf("aa000000000000000000000000%06x", n)
 }
 
 // ResetPlaceholderCounter resets the counter (for testing).
 func ResetPlaceholderCounter() {
-	placeholderCounter = 0
+	placeholderCounter.Store(0)
 }
 
 // getMapField gets a nested map field from a JSON map.