fix: address PR mendixlabs#335 review feedback

- Remove [perf] log noise from Store.ListUnits (#1)
- Fix FindByQualifiedName docs: clarify it matches simple Name, not qualified (#2)
- Fix concurrency docs: "not safe for concurrent mutation" (#3)
- Add TODO for WriteTransaction.Commit partial-failure window (#4)
- Add TODO for PatchEncoded escape-hatch methods (#5)
- Add path traversal guard in DeleteModuleWithCleanup (#6)
- Add package doc.go with architecture overview and migration guidance (#7)

Author: claude

modelsdk/codec/store.go

@@ -2,7 +2,6 @@ package codec
 
 import (
 	"fmt"
-	"log"
 	"time"
 
 	"github.com/mendixlabs/mxcli/modelsdk/element"
@@ -87,7 +86,7 @@ func (s *Store) ListUnits() []UnitInfo {
 			Name:        name,
 		})
 	}
-	log.Printf("[perf] Store.ListUnits: %s (units=%d)", time.Since(listStart), len(units))
+	_ = listStart // reserved for future debug logging
 	return units
 }
 

modelsdk/doc.go

@@ -0,0 +1,36 @@
+// Package modelsdk provides auto-generated, type-safe access to Mendix MPR files.
+//
+// modelsdk is designed as a replacement for the hand-written sdk/ package.
+// It coexists with sdk/ during migration — no existing code is modified.
+//
+// # Choosing between modelsdk/ and sdk/
+//
+// Use modelsdk/ for new code that needs:
+//   - Dirty tracking (know what changed before writing)
+//   - BSON roundtrip safety (unimplemented fields survive read/write)
+//   - Coverage beyond the 5-10 domains in sdk/ (53 domains here)
+//
+// Use sdk/ for existing code that already works — migrate incrementally.
+//
+// # Architecture
+//
+// The package has three layers:
+//
+//   - element.Base: identity, raw BSON bytes, dirty bitmap with container chain propagation
+//   - property.Primitive/Part/PartList/Enum/ByNameRef: lazy-decoded fields with per-bit dirty tracking
+//   - codec.Decoder/Encoder: type registry dispatch with three-branch encoding (clean/child-dirty/self-dirty)
+//
+// # Generated code
+//
+// The modelsdk/gen/ subdirectory contains auto-generated types for all 53 Mendix
+// metamodel domains. To regenerate after updating the TypeScript SDK reference:
+//
+//	npm install mendixmodelsdk --prefix reference/mendixmodelsdk
+//	go run ./cmd/modelsdk-codegen
+//
+// Each generated domain package contains:
+//   - types.go: struct definitions, getters/setters, InitFromRaw, registry init()
+//   - enums.go: enumeration type aliases and constants
+//   - refs.go: cross-domain reference metadata registration
+//   - version.go: per-property introduced/deleted/public version info
+package modelsdk

modelsdk/model.go

@@ -83,7 +83,7 @@ func (m *Model) Units() []codec.UnitInfo {
 }
 
 // LoadUnit loads and decodes a single unit by ID. Results are cached.
-// Safe for concurrent use.
+// Not safe for concurrent mutation — use from a single goroutine.
 func (m *Model) LoadUnit(id element.ID) (element.Element, error) {
 	if elem, ok := m.uc.Get(id); ok {
 		return elem, nil
@@ -104,7 +104,7 @@ func (m *Model) LoadUnit(id element.ID) (element.Element, error) {
 
 // AllOfType loads and returns all units whose $Type matches the given type name.
 // Uses UnitInfo.Type metadata to pre-filter, avoiding BSON I/O for non-matching units.
-// Safe for concurrent use.
+// Not safe for concurrent mutation — use from a single goroutine.
 func (m *Model) AllOfType(typeName string) []element.Element {
 	var result []element.Element
 	currentTypeGen := m.uc.TypeGen(typeName)
@@ -154,12 +154,13 @@ func (m *Model) AllOfType(typeName string) []element.Element {
 	return result
 }
 
-// FindByQualifiedName searches for a unit of the given type whose
-// "Name" property matches qualifiedName. Uses the name index for O(1)
-// lookup, falling back to a linear scan if the index misses.
-func (m *Model) FindByQualifiedName(typeName, qualifiedName string) (element.Element, error) {
+// FindByQualifiedName searches for a unit of the given type whose BSON
+// "Name" field matches name. Note: Mendix stores the module-local simple
+// name (e.g. "ACT_GetUser"), not the dot-qualified name ("MyModule.ACT_GetUser").
+// Uses the name index for O(1) lookup, falling back to a linear scan if the index misses.
+func (m *Model) FindByQualifiedName(typeName, name string) (element.Element, error) {
 	// Fast path: index lookup.
-	if id, ok := m.uc.FindByName(typeName + ":" + qualifiedName); ok {
+	if id, ok := m.uc.FindByName(typeName + ":" + name); ok {
 		return m.LoadUnit(id)
 	}
 
@@ -175,8 +176,8 @@ func (m *Model) FindByQualifiedName(typeName, qualifiedName string) (element.Ele
 		if u.Type == "" && decodeTypeField(raw) != typeName {
 			continue
 		}
-		name, _ := raw.LookupErr("Name")
-		if s, ok := name.StringValueOK(); ok && s == qualifiedName {
+		bsonName, _ := raw.LookupErr("Name")
+		if s, ok := bsonName.StringValueOK(); ok && s == name {
 			elem, err := m.decoder.Decode(raw)
 			if err != nil {
 				return nil, fmt.Errorf("decode unit %s: %w", u.ID, err)
@@ -185,7 +186,7 @@ func (m *Model) FindByQualifiedName(typeName, qualifiedName string) (element.Ele
 			return elem, nil
 		}
 	}
-	return nil, fmt.Errorf("element %s with name %q not found", typeName, qualifiedName)
+	return nil, fmt.Errorf("element %s with name %q not found", typeName, name)
 }
 
 // Encode serializes an element to BSON bytes.
@@ -238,7 +239,8 @@ func (m *Model) Store() *codec.Store { return m.store }
 // PatchEncodedField sets a top-level field on already-encoded BSON bytes.
 // Use this only when the SDK type's setter has a different type than the
 // BSON storage format (e.g., SDK uses Part[element.Element] but BSON stores
-// a plain string). This is a last-resort escape hatch.
+// a plain string).
+// TODO: remove when generated setters cover all BSON storage type mismatches.
 func (m *Model) PatchEncodedField(data []byte, key string, value any) ([]byte, error) {
 	return codec.PatchBSONField(data, key, value)
 }
@@ -342,9 +344,13 @@ func (m *Model) DeleteModuleWithCleanup(moduleID element.ID, moduleName string)
 
 	// Clean up themesource directory.
 	projectDir := filepath.Dir(m.store.Path())
-	themesourceDir := filepath.Join(projectDir, "themesource", strings.ToLower(moduleName))
-	if stat, err := os.Stat(themesourceDir); err == nil && stat.IsDir() {
-		os.RemoveAll(themesourceDir)
+	themesourceBase := filepath.Join(projectDir, "themesource")
+	themesourceDir := filepath.Clean(filepath.Join(themesourceBase, strings.ToLower(moduleName)))
+	// Guard against path traversal: the resolved path must be under themesource/.
+	if strings.HasPrefix(themesourceDir, themesourceBase+string(filepath.Separator)) {
+		if stat, err := os.Stat(themesourceDir); err == nil && stat.IsDir() {
+			os.RemoveAll(themesourceDir)
+		}
 	}
 
 	return nil

modelsdk/mpr/writer_core.go

@@ -157,6 +157,8 @@ func (wt *WriteTransaction) WriteUnit(unitID string, contents []byte) error {
 
 // Commit commits the transaction.
 // For v2, this first commits the database, then finalizes file writes.
+// TODO: adopt two-phase approach (rename files first, then commit DB) to
+// eliminate the partial-failure window where DB is committed but files are stale.
 func (wt *WriteTransaction) Commit() error {
 	if wt.committed {
 		return fmt.Errorf("transaction already committed")