refactor: quick-win cleanup — panic stubs, SaveToFile, dead code, error handling, syncWriter

Author: retran

mdl/backend/mpr/backend.go

@@ -768,17 +768,17 @@ func (b *MprBackend) OpenWorkflowForMutation(unitID model.ID) (backend.WorkflowM
 // WidgetSerializationBackend
 
 func (b *MprBackend) SerializeWidget(w pages.Widget) (any, error) {
-	panic("MprBackend.SerializeWidget not yet implemented") // TODO: implement in PR #237
+	return mpr.SerializeWidget(w), nil
 }
 
 func (b *MprBackend) SerializeClientAction(a pages.ClientAction) (any, error) {
-	panic("MprBackend.SerializeClientAction not yet implemented") // TODO: implement in PR #237
+	return mpr.SerializeClientAction(a), nil
 }
 
 func (b *MprBackend) SerializeDataSource(ds pages.DataSource) (any, error) {
 	return mpr.SerializeCustomWidgetDataSource(ds), nil
 }
 
 func (b *MprBackend) SerializeWorkflowActivity(a workflows.WorkflowActivity) (any, error) {
-	panic("MprBackend.SerializeWorkflowActivity not yet implemented") // TODO: implement in PR #237
+	return mpr.SerializeWorkflowActivity(a), nil
 }

mdl/catalog/builder.go

@@ -79,8 +79,8 @@ type Builder struct {
 	progress     ProgressFunc
 	hierarchy    *hierarchy
 	tx           CatalogTx // Transaction for batched inserts
-	fullMode     bool    // If true, do full parsing (activities/widgets)
-	sourceMode   bool    // If true, build source FTS table (implies full)
+	fullMode     bool      // If true, do full parsing (activities/widgets)
+	sourceMode   bool      // If true, build source FTS table (implies full)
 	describeFunc DescribeFunc
 
 	// Document caches — avoid redundant BSON parsing across builder phases.

mdl/catalog/builder_microflows_test.go

@@ -5,8 +5,8 @@ package catalog
 import (
 	"testing"
 
-	"github.com/mendixlabs/mxcli/sdk/microflows"
 	"github.com/mendixlabs/mxcli/model"
+	"github.com/mendixlabs/mxcli/sdk/microflows"
 )
 
 // unknownMicroflowObject satisfies MicroflowObject but is not in the type switch.

mdl/catalog/builder_pages.go

@@ -5,6 +5,7 @@ package catalog
 import (
 	"database/sql"
 	"encoding/base64"
+	"fmt"
 	"strings"
 
 	"go.mongodb.org/mongo-driver/bson/primitive"
@@ -103,7 +104,7 @@ func (b *Builder) buildPages() error {
 		// Insert widgets only in full mode
 		if b.fullMode && len(rawWidgets) > 0 {
 			for _, w := range rawWidgets {
-				_, _ = widgetStmt.Exec(
+				if _, err := widgetStmt.Exec(
 					w.ID,
 					w.Name,
 					w.WidgetType,
@@ -117,7 +118,9 @@ func (b *Builder) buildPages() error {
 					"",
 					projectID, projectName, snapshotID, snapshotDate, snapshotSource,
 					sourceID, sourceBranch, sourceRevision,
-				)
+				); err != nil {
+					return fmt.Errorf("insert widget %s for page %s: %w", w.Name, qualifiedName, err)
+				}
 				widgetCount++
 			}
 		}

mdl/catalog/catalog.go

@@ -284,16 +284,10 @@ func (c *Catalog) SaveToFile(path string) error {
 	}
 	rawDB := sdb.RawDB()
 
-	// Open destination file database
-	destDB, err := sql.Open("sqlite", path)
-	if err != nil {
-		return fmt.Errorf("failed to create catalog file: %w", err)
-	}
-	defer destDB.Close()
-
 	// Use SQLite backup API via VACUUM INTO (SQLite 3.27+)
 	// Fall back to manual copy if not available
-	_, err = rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", path))
+	safePath := strings.ReplaceAll(path, "'", "''")
+	_, err := rawDB.Exec(fmt.Sprintf("VACUUM INTO '%s'", safePath))
 	if err != nil {
 		// Fall back: export and import
 		return c.saveToFileManual(path, rawDB)

mdl/executor/cmd_catalog.go

@@ -7,9 +7,12 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
+	"log"
 	"os"
 	"path/filepath"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
@@ -18,6 +21,19 @@ import (
 	"github.com/mendixlabs/mxcli/model"
 )
 
+// syncWriter wraps an io.Writer with a mutex so concurrent goroutines
+// (e.g. background catalog build) can write without racing.
+type syncWriter struct {
+	mu sync.Mutex
+	w  io.Writer
+}
+
+func (sw *syncWriter) Write(p []byte) (int, error) {
+	sw.mu.Lock()
+	defer sw.mu.Unlock()
+	return sw.w.Write(p)
+}
+
 // execShowCatalogTables handles SHOW CATALOG TABLES.
 func execShowCatalogTables(ctx *ExecContext) error {
 	// Build catalog if not already built (fast mode by default)
@@ -449,19 +465,29 @@ func execRefreshCatalogStmt(ctx *ExecContext, stmt *ast.RefreshCatalogStmt) erro
 
 	// Handle background mode — clone ctx so the goroutine doesn't race
 	// with the main dispatch loop (which may syncBack and mutate fields).
-	// NOTE: bgCtx.Output still shares the underlying writer with the main
-	// goroutine. This is a pre-existing limitation — the original code also
-	// wrote to ctx.Output from the goroutine. A synchronized writer would
-	// fix this but is out of scope for the executor cleanup.
 	if stmt.Background {
-		bgCtx := *ctx   // shallow copy — isolates scalar fields
-		bgCtx.Cache = nil // detach shared cache so preWarmCache writes stay local
+		bgCtx := *ctx                    // shallow copy — isolates scalar fields
+		bgCtx.Cache = nil                // detach shared cache so preWarmCache writes stay local
+		sw := &syncWriter{w: ctx.Output} // shared mutex-wrapped writer
+		bgCtx.Output = sw                // background goroutine writes through sw
+		syncCatalog := ctx.SyncCatalog   // capture callback before returning
 		go func() {
 			if err := buildCatalog(&bgCtx, stmt.Full, stmt.Source); err != nil {
 				fmt.Fprintf(bgCtx.Output, "Background catalog build failed: %v\n", err)
+				return
+			}
+			// Propagate the built catalog back to the Executor so the next
+			// command picks it up. syncBack has already run for this statement,
+			// so we use the SyncCatalog callback to write directly to e.catalog.
+			if bgCtx.Catalog != nil {
+				if syncCatalog != nil {
+					syncCatalog(bgCtx.Catalog)
+				} else {
+					bgCtx.Catalog.Close()
+				}
 			}
 		}()
-		fmt.Fprintln(ctx.Output, "Catalog build started in background...")
+		fmt.Fprintln(sw, "Catalog build started in background...")
 		return nil
 	}
 
@@ -618,7 +644,9 @@ func outputCatalogResults(ctx *ExecContext, result *catalog.QueryResult) {
 		}
 		tr.Rows = append(tr.Rows, outRow)
 	}
-	_ = writeResult(ctx, tr)
+	if err := writeResult(ctx, tr); err != nil {
+		log.Printf("warning: failed to write catalog results: %v", err)
+	}
 }
 
 // formatValue formats a value for display.

mdl/executor/cmd_microflows_builder.go

@@ -23,25 +23,25 @@ type flowBuilder struct {
 	posY                int
 	baseY               int // Base Y position (for returning after ELSE branches)
 	spacing             int
-	returnValue         string                       // Return value expression for RETURN statement (used by buildFlowGraph final EndEvent)
-	endsWithReturn      bool                         // True if the flow already ends with EndEvent(s) from RETURN statements
-	varTypes            map[string]string            // Variable name -> entity qualified name (for CHANGE statements)
-	declaredVars        map[string]string            // Declared primitive variables: name -> type (e.g., "$IsValid" -> "Boolean")
-	errors              []string                     // Validation errors collected during build
-	measurer            *layoutMeasurer              // For measuring statement dimensions
-	nextConnectionPoint model.ID                     // For compound statements: the exit point differs from entry point
-	nextFlowCase        string                       // If set, next connecting flow uses this case value (for merge-less splits)
+	returnValue         string            // Return value expression for RETURN statement (used by buildFlowGraph final EndEvent)
+	endsWithReturn      bool              // True if the flow already ends with EndEvent(s) from RETURN statements
+	varTypes            map[string]string // Variable name -> entity qualified name (for CHANGE statements)
+	declaredVars        map[string]string // Declared primitive variables: name -> type (e.g., "$IsValid" -> "Boolean")
+	errors              []string          // Validation errors collected during build
+	measurer            *layoutMeasurer   // For measuring statement dimensions
+	nextConnectionPoint model.ID          // For compound statements: the exit point differs from entry point
+	nextFlowCase        string            // If set, next connecting flow uses this case value (for merge-less splits)
 	// nextFlowAnchor carries the branch-specific FlowAnchors that should be
 	// applied to the flow created by the NEXT iteration of buildFlowGraph.
 	// Used by guard-pattern IFs (where one branch returns and the other
 	// continues) so the continuing branch's @anchor survives to the actual
 	// splitID→nextActivity flow — which is emitted one iteration later by the
 	// outer loop, not by addIfStatement.
-	nextFlowAnchor      *ast.FlowAnchors
-	backend             backend.FullBackend          // For looking up page/microflow references
-	hierarchy           *ContainerHierarchy          // For resolving container IDs to module names
-	pendingAnnotations  *ast.ActivityAnnotations     // Pending annotations to attach to next activity
-	restServices        []*model.ConsumedRestService // Cached REST services for parameter classification
+	nextFlowAnchor     *ast.FlowAnchors
+	backend            backend.FullBackend          // For looking up page/microflow references
+	hierarchy          *ContainerHierarchy          // For resolving container IDs to module names
+	pendingAnnotations *ast.ActivityAnnotations     // Pending annotations to attach to next activity
+	restServices       []*model.ConsumedRestService // Cached REST services for parameter classification
 	// previousStmtAnchor holds the Anchor annotation of the statement that
 	// just emitted an activity, so the next flow's OriginConnectionIndex can
 	// be overridden by the user. Cleared after each flow is created.

mdl/executor/cmd_microflows_builder_calls.go

@@ -5,6 +5,7 @@ package executor
 
 import (
 	"fmt"
+	"log"
 	"strings"
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
@@ -174,7 +175,11 @@ func (fb *flowBuilder) addCallJavaActionAction(s *ast.CallJavaActionStmt) model.
 	// Try to look up the Java action definition to detect EntityTypeParameterType parameters
 	var jaDef *javaactions.JavaAction
 	if fb.backend != nil {
-		jaDef, _ = fb.backend.ReadJavaActionByName(actionQN)
+		var err error
+		jaDef, err = fb.backend.ReadJavaActionByName(actionQN)
+		if err != nil {
+			log.Printf("warning: could not look up Java action %s: %v (entity type params will be empty)", actionQN, err)
+		}
 	}
 
 	// Build a map of parameter name -> param type for the Java action

mdl/executor/cmd_page_wireframe.go

@@ -6,6 +6,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"log"
 	"strings"
 
 	"github.com/mendixlabs/mxcli/mdl/ast"
@@ -485,7 +486,9 @@ func pageToMdlString(ctx *ExecContext, name ast.QualifiedName, root []wireframeN
 	var buf strings.Builder
 	origOutput := ctx.Output
 	ctx.Output = &buf
-	_ = describePage(ctx, name)
+	if err := describePage(ctx, name); err != nil {
+		log.Printf("warning: describePage failed for %s.%s: %v", name.Module, name.Name, err)
+	}
 	ctx.Output = origOutput
 
 	mdlSource := buf.String()

mdl/executor/cmd_pages_builder_v3.go

@@ -491,7 +491,11 @@ func (pb *pageBuilder) buildDataSourceV3(ds *ast.DataSourceV3) (pages.DataSource
 
 		// Fallback to lookup if entity name not stored
 		if entityName == "" {
-			entityName, _ = pb.getEntityNameByID(entityID)
+			var err error
+			entityName, err = pb.getEntityNameByID(entityID)
+			if err != nil {
+				log.Printf("warning: could not resolve entity name for ID %s: %v", entityID, err)
+			}
 		}
 
 		// Use DataViewSource with IsSnippetParameter flag