From 9a807c3d27e3dc997f96fd8fe5ef5314ec4d5b87 Mon Sep 17 00:00:00 2001 From: Ylber Date: Thu, 16 Apr 2026 17:00:40 +0200 Subject: [PATCH 1/3] =?UTF-8?q?docs:=20add=20fix-issue=20skill=20with=20TD?= =?UTF-8?q?D=20protocol=20and=20symptom=E2=86=92layer=20table?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces .claude/skills/fix-issue.md — a shared skill for diagnosing and fixing bugs faster. Seeds the symptom table from issue #212. Each fix appends a new row, compounding over time. Wires the skill into the PR checklist in CLAUDE.md under a new Bug fixes section. Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/fix-issue.md | 76 +++++++++++++++++++++++++++++++++++++ CLAUDE.md | 5 +++ 2 files changed, 81 insertions(+) create mode 100644 .claude/skills/fix-issue.md diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md new file mode 100644 index 00000000..695f0be1 --- /dev/null +++ b/.claude/skills/fix-issue.md @@ -0,0 +1,76 @@ +# Fix Issue Skill + +A fast-path workflow for diagnosing and fixing bugs in mxcli. Each fix appends +to the symptom table below, so the next similar issue costs fewer reads. + +## How to Use + +1. Match the issue symptom to a row in the table — go straight to that file. +2. Follow the fix pattern for that row. +3. Write a failing test first, then implement. +4. After the fix: **add a new row** to the table if the symptom is not already covered. + +--- + +## Symptom → Layer → File Table + +| Symptom | Root cause layer | First file to open | Fix pattern | +|---------|-----------------|-------------------|-------------| +| `DESCRIBE` shows `$var = LIST OPERATION ...;` | Missing parser case | `sdk/mpr/parser_microflow.go` → `parseListOperation()` | Add `case "Microflows$XxxType":` returning the correct struct | +| `DESCRIBE` shows `$var = ACTION ...;` | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatActionStatement()` | Add `case *microflows.XxxAction:` with `fmt.Sprintf` output | +| `DESCRIBE` shows `$var = LIST OPERATION %T;` (with type name) | Missing formatter case | `mdl/executor/cmd_microflows_format_action.go` → `formatListOperation()` | Add `case *microflows.XxxOperation:` before the `default` | +| Compile error: `undefined: microflows.XxxOperation` | Missing SDK struct | `sdk/microflows/microflows_actions.go` | Add struct + `func (XxxOperation) isListOperation() {}` marker | +| `TypeCacheUnknownTypeException` in Studio Pro | Wrong `$Type` storage name in BSON write | `sdk/mpr/writer_microflow.go` | Check the storage name table in CLAUDE.md; verify against `reference/mendixmodellib/reflection-data/` | +| CE0066 "Entity access is out of date" | MemberAccess added to wrong entity | `sdk/mpr/writer_domainmodel.go` | MemberAccess must only be on the FROM entity (`ParentPointer`), not the TO entity — see CLAUDE.md association semantics | +| CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` | +| Parser returns `nil` for a known BSON type | Unhandled `default` in a `parseXxx()` switch | `sdk/mpr/parser_microflow.go` or `parser_page.go` | Find the switch by grepping for `default: return nil`; add the missing case | +| MDL check gives "unexpected token" on valid-looking syntax | Grammar missing rule or token | `mdl/grammar/MDLParser.g4` + `MDLLexer.g4` | Add rule/token, run `make grammar` | + +--- + +## TDD Protocol + +Always follow this order — never implement before the test exists: + +``` +Step 1: Write a failing unit test (parser test or formatter test) +Step 2: Confirm it fails to compile or fails at runtime +Step 3: Implement the minimum code to make it pass +Step 4: Run: /c/Users/Ylber.Sadiku/go/go/bin/go test ./mdl/executor/... ./sdk/mpr/... +Step 5: Add the symptom row to the table above if not already present +``` + +Parser tests go in `sdk/mpr/parser__test.go`. +Formatter tests go in `mdl/executor/cmd__format__test.go`. + +--- + +## Issue #212 — Reference Fix (seeding example) + +**Symptom:** `DESCRIBE MICROFLOW` showed `$var = LIST OPERATION ...;` for +`Microflows$Find`, `Microflows$Filter`, `Microflows$ListRange`. + +**Root cause:** `parseListOperation()` in `sdk/mpr/parser_microflow.go` had no +cases for these three BSON types — they fell to `default: return nil`. + +**Files changed:** +| File | Change | +|------|--------| +| `sdk/microflows/microflows_actions.go` | Added `FindByAttributeOperation`, `FilterByAttributeOperation`, `RangeOperation` | +| `sdk/mpr/parser_microflow.go` | Added 3 parser cases | +| `mdl/executor/cmd_microflows_format_action.go` | Added 3 formatter cases | +| `mdl/executor/cmd_microflows_format_listop_test.go` | Added 4 formatter tests | +| `sdk/mpr/parser_listoperation_test.go` | New file, 4 parser tests | + +**Key insight:** `Microflows$ListRange` stores offset/limit inside a nested +`CustomRange` map — must cast `raw["CustomRange"].(map[string]any)` before +extracting `OffsetExpression`/`LimitExpression`. + +--- + +## After Every Fix — Checklist + +- [ ] Failing test written before implementation +- [ ] `go test ./mdl/executor/... ./sdk/mpr/...` passes +- [ ] New symptom row added to the table above (if not already covered) +- [ ] PR title: `fix: ` diff --git a/CLAUDE.md b/CLAUDE.md index 8cbad082..d1c19dcc 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -224,6 +224,11 @@ Available namespaces: `DomainModels`, `Enumerations`, `Microflows`, `Pages`, `Mo When reviewing pull requests or validating work before commit, verify these items: +### Bug fixes +- [ ] **Fix-issue skill consulted** — read `.claude/skills/fix-issue.md` before diagnosing; match symptom to table before opening files +- [ ] **Symptom table updated** — new symptom/layer/file mapping added to `.claude/skills/fix-issue.md` if not already covered +- [ ] **Test written first** — failing test exists before implementation (parser test in `sdk/mpr/`, formatter test in `mdl/executor/`) + ### Overlap & duplication - [ ] Check `docs/11-proposals/` for existing proposals covering the same functionality - [ ] Search the codebase for existing implementations (grep for key function names, command names, types) From 80dffe38ffef0084b760f541d0fbe9cb19ff5e14 Mon Sep 17 00:00:00 2001 From: Ylber Date: Thu, 16 Apr 2026 17:31:27 +0200 Subject: [PATCH 2/3] fix: route path/query params correctly and suppress BodyVariable for JSON bodies in SEND REST REQUEST Fixes CE7054 ("parameters updated") and CE7067 ("does not support body entity") errors produced by mx check after mxcli generates a Microflows$RestOperationCallAction. Two root causes: - All WITH-clause params were emitted as QueryParameterMappings regardless of whether they are path or query params on the operation definition. - BodyVariable was always serialised when BODY $var was present, but for Rest$JsonBody / Rest$StringBody operations the body lives on the operation template and BodyVariable must be nil. Fix: look up the ConsumedRestService operation at build time via a new restServices field on flowBuilder (populated from loadRestServices()), then use lookupRestOperation / buildRestParameterMappings / shouldSetBodyVariable helpers to emit the correct BSON. When the operation is not found we fall back to the previous behaviour. Closes #193 Co-Authored-By: Claude Sonnet 4.6 --- .claude/skills/fix-issue.md | 1 + mdl/executor/cmd_microflows_builder.go | 1 + mdl/executor/cmd_microflows_builder_calls.go | 111 ++++++-- .../cmd_microflows_builder_control.go | 2 + mdl/executor/cmd_microflows_builder_flows.go | 1 + .../cmd_microflows_builder_rest_test.go | 262 ++++++++++++++++++ mdl/executor/cmd_microflows_create.go | 12 + 7 files changed, 374 insertions(+), 16 deletions(-) create mode 100644 mdl/executor/cmd_microflows_builder_rest_test.go diff --git a/.claude/skills/fix-issue.md b/.claude/skills/fix-issue.md index 695f0be1..f2b2c456 100644 --- a/.claude/skills/fix-issue.md +++ b/.claude/skills/fix-issue.md @@ -25,6 +25,7 @@ to the symptom table below, so the next similar issue costs fewer reads. | CE0463 "widget definition changed" | Object property structure doesn't match Type PropertyTypes | `sdk/widgets/templates/` | Re-extract template from Studio Pro; see `sdk/widgets/templates/README.md` | | Parser returns `nil` for a known BSON type | Unhandled `default` in a `parseXxx()` switch | `sdk/mpr/parser_microflow.go` or `parser_page.go` | Find the switch by grepping for `default: return nil`; add the missing case | | MDL check gives "unexpected token" on valid-looking syntax | Grammar missing rule or token | `mdl/grammar/MDLParser.g4` + `MDLLexer.g4` | Add rule/token, run `make grammar` | +| CE7054 "parameters updated" / CE7067 "does not support body entity" after `SEND REST REQUEST` | `addSendRestRequestAction` emitted wrong BSON: all params as query params, BodyVariable set for JSON bodies | `mdl/executor/cmd_microflows_builder_calls.go` → `addSendRestRequestAction` | Look up operation via `fb.restServices`; route path/query params with `buildRestParameterMappings`; suppress BodyVariable for JSON/TEMPLATE/FILE via `shouldSetBodyVariable` | --- diff --git a/mdl/executor/cmd_microflows_builder.go b/mdl/executor/cmd_microflows_builder.go index f0c0a2d7..130e4f9f 100644 --- a/mdl/executor/cmd_microflows_builder.go +++ b/mdl/executor/cmd_microflows_builder.go @@ -33,6 +33,7 @@ type flowBuilder struct { reader *mpr.Reader // 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 } // addError records a validation error during flow building. diff --git a/mdl/executor/cmd_microflows_builder_calls.go b/mdl/executor/cmd_microflows_builder_calls.go index f52d59d0..94a8db0a 100644 --- a/mdl/executor/cmd_microflows_builder_calls.go +++ b/mdl/executor/cmd_microflows_builder_calls.go @@ -769,6 +769,16 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode // Build operation reference: Module.Service.Operation operationQN := s.Operation.String() + // Look up the operation definition to classify parameters and body kind. + // s.Operation.Module = "MfTest", s.Operation.Name = "RC_TestApi.PostJsonTemplate" + var opDef *model.RestClientOperation + if fb.restServices != nil && s.Operation.Module != "" && strings.Contains(s.Operation.Name, ".") { + dotIdx := strings.Index(s.Operation.Name, ".") + serviceName := s.Operation.Name[:dotIdx] + opName := s.Operation.Name[dotIdx+1:] + opDef = lookupRestOperation(fb.restServices, serviceName, opName) + } + // Build OutputVariable var outputVar *microflows.RestOutputVar if s.OutputVariable != "" { @@ -778,29 +788,20 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode } } - // Build BodyVariable + // Build BodyVariable only for EXPORT_MAPPING body kind. + // For JSON / TEMPLATE / FILE bodies, the body expression lives on the + // operation definition itself and must NOT be set here (CE7067). var bodyVar *microflows.RestBodyVar - if s.BodyVariable != "" { + if s.BodyVariable != "" && shouldSetBodyVariable(opDef) { bodyVar = µflows.RestBodyVar{ BaseElement: model.BaseElement{ID: model.ID(mpr.GenerateID())}, VariableName: s.BodyVariable, } } - // Build parameter mappings from WITH clause - var paramMappings []*microflows.RestParameterMapping - var queryParamMappings []*microflows.RestQueryParameterMapping - for _, p := range s.Parameters { - // Determine if path or query param by convention: - // the executor can't distinguish at this level, so we emit both - // and let the BSON field names sort it out. For now, emit as - // query parameter mappings (most common use case). - queryParamMappings = append(queryParamMappings, µflows.RestQueryParameterMapping{ - Parameter: operationQN + "." + p.Name, - Value: p.Expression, - Included: "Yes", - }) - } + // Build parameter mappings, routing to ParameterMappings (path) or + // QueryParameterMappings (query) based on the operation definition. + paramMappings, queryParamMappings := buildRestParameterMappings(s.Parameters, opDef, operationQN) // RestOperationCallAction does not support custom error handling (CE6035). // ON ERROR clauses in the MDL are silently ignored for this action type. @@ -831,6 +832,84 @@ func (fb *flowBuilder) addSendRestRequestAction(s *ast.SendRestRequestStmt) mode return activity.ID } +// lookupRestOperation finds a specific operation in a consumed REST service list. +func lookupRestOperation(services []*model.ConsumedRestService, serviceName, opName string) *model.RestClientOperation { + for _, svc := range services { + if svc.Name != serviceName { + continue + } + for _, op := range svc.Operations { + if op.Name == opName { + return op + } + } + } + return nil +} + +// shouldSetBodyVariable returns true if a BodyVariable BSON field should be +// emitted for a call to the given operation. +// For JSON, TEMPLATE, and FILE body kinds, the body expression lives on the +// operation definition and must not be overridden by a BodyVariable (CE7067). +// For EXPORT_MAPPING, the caller provides an entity to export via BodyVariable. +// When the operation definition is unknown (nil), we preserve old behaviour and +// set BodyVariable so the caller's intent is not silently dropped. +func shouldSetBodyVariable(op *model.RestClientOperation) bool { + if op == nil { + return true // unknown operation — preserve caller intent + } + switch op.BodyType { + case "JSON", "TEMPLATE", "FILE": + return false + default: + // EXPORT_MAPPING or empty (no body) — only set if EXPORT_MAPPING + return op.BodyType == "EXPORT_MAPPING" + } +} + +// buildRestParameterMappings splits parameter bindings from a SEND REST REQUEST +// WITH clause into path parameter mappings and query parameter mappings, +// using the operation definition to determine which is which. +// When op is nil (operation not found), all parameters fall back to query +// parameter mappings (preserves old behaviour). +func buildRestParameterMappings( + params []ast.SendRestParamDef, + op *model.RestClientOperation, + operationQN string, +) ([]*microflows.RestParameterMapping, []*microflows.RestQueryParameterMapping) { + if len(params) == 0 { + return nil, nil + } + + // Build lookup sets from the operation definition. + pathParamSet := map[string]bool{} + if op != nil { + for _, p := range op.Parameters { + pathParamSet[p.Name] = true + } + } + + var pathMappings []*microflows.RestParameterMapping + var queryMappings []*microflows.RestQueryParameterMapping + + for _, p := range params { + if pathParamSet[p.Name] { + pathMappings = append(pathMappings, µflows.RestParameterMapping{ + Parameter: operationQN + "." + p.Name, + Value: p.Expression, + }) + } else { + queryMappings = append(queryMappings, µflows.RestQueryParameterMapping{ + Parameter: operationQN + "." + p.Name, + Value: p.Expression, + Included: "Yes", + }) + } + } + + return pathMappings, queryMappings +} + // addExecuteDatabaseQueryAction creates an EXECUTE DATABASE QUERY statement. func (fb *flowBuilder) addExecuteDatabaseQueryAction(s *ast.ExecuteDatabaseQueryStmt) model.ID { // DynamicQuery is a Mendix expression — string literals need single quotes diff --git a/mdl/executor/cmd_microflows_builder_control.go b/mdl/executor/cmd_microflows_builder_control.go index 5ccde912..6191bbd3 100644 --- a/mdl/executor/cmd_microflows_builder_control.go +++ b/mdl/executor/cmd_microflows_builder_control.go @@ -287,6 +287,7 @@ func (fb *flowBuilder) addLoopStatement(s *ast.LoopStmt) model.ID { measurer: fb.measurer, // Share measurer reader: fb.reader, // Share reader hierarchy: fb.hierarchy, // Share hierarchy + restServices: fb.restServices, // Share REST services for parameter classification } // Process loop body statements and connect them with flows @@ -360,6 +361,7 @@ func (fb *flowBuilder) addWhileStatement(s *ast.WhileStmt) model.ID { measurer: fb.measurer, reader: fb.reader, hierarchy: fb.hierarchy, + restServices: fb.restServices, } var lastBodyID model.ID diff --git a/mdl/executor/cmd_microflows_builder_flows.go b/mdl/executor/cmd_microflows_builder_flows.go index ce90f615..f0d554a1 100644 --- a/mdl/executor/cmd_microflows_builder_flows.go +++ b/mdl/executor/cmd_microflows_builder_flows.go @@ -66,6 +66,7 @@ func (fb *flowBuilder) addErrorHandlerFlow(sourceActivityID model.ID, sourceX in measurer: fb.measurer, reader: fb.reader, hierarchy: fb.hierarchy, + restServices: fb.restServices, } var lastErrID model.ID diff --git a/mdl/executor/cmd_microflows_builder_rest_test.go b/mdl/executor/cmd_microflows_builder_rest_test.go new file mode 100644 index 00000000..0b232f42 --- /dev/null +++ b/mdl/executor/cmd_microflows_builder_rest_test.go @@ -0,0 +1,262 @@ +// SPDX-License-Identifier: Apache-2.0 + +package executor + +import ( + "testing" + + "github.com/mendixlabs/mxcli/mdl/ast" + "github.com/mendixlabs/mxcli/model" + "github.com/mendixlabs/mxcli/sdk/microflows" +) + +// --- lookupRestOperation helper tests --- + +func TestLookupRestOperation_Found(t *testing.T) { + svc := &model.ConsumedRestService{ + BaseElement: model.BaseElement{ID: "svc-1"}, + Name: "MyAPI", + Operations: []*model.RestClientOperation{ + {Name: "PostData", BodyType: "JSON"}, + }, + } + op := lookupRestOperation([]*model.ConsumedRestService{svc}, "MyAPI", "PostData") + if op == nil { + t.Fatal("expected operation to be found, got nil") + } + if op.Name != "PostData" { + t.Errorf("got op.Name=%q, want PostData", op.Name) + } +} + +func TestLookupRestOperation_NotFound(t *testing.T) { + svc := &model.ConsumedRestService{ + BaseElement: model.BaseElement{ID: "svc-1"}, + Name: "MyAPI", + Operations: []*model.RestClientOperation{ + {Name: "GetItems", BodyType: ""}, + }, + } + op := lookupRestOperation([]*model.ConsumedRestService{svc}, "MyAPI", "PostData") + if op != nil { + t.Errorf("expected nil for unknown operation, got %+v", op) + } +} + +// --- buildRestParameterMappings helper tests --- + +// Test: operation has path param $userId and query param $status. +// With clause binds both. Expect path in ParameterMappings, query in QueryParameterMappings. +func TestBuildRestParameterMappings_PathAndQuery(t *testing.T) { + op := &model.RestClientOperation{ + Name: "GetUser", + Parameters: []*model.RestClientParameter{ + {Name: "userId", DataType: "Integer"}, + }, + QueryParameters: []*model.RestClientParameter{ + {Name: "status", DataType: "String"}, + }, + } + opQN := "Test.MyAPI.GetUser" + + params := []ast.SendRestParamDef{ + {Name: "userId", Expression: "$UserId"}, + {Name: "status", Expression: "'active'"}, + } + + pathMappings, queryMappings := buildRestParameterMappings(params, op, opQN) + + if len(pathMappings) != 1 { + t.Fatalf("expected 1 path mapping, got %d", len(pathMappings)) + } + if pathMappings[0].Parameter != "Test.MyAPI.GetUser.userId" { + t.Errorf("got Parameter=%q, want Test.MyAPI.GetUser.userId", pathMappings[0].Parameter) + } + if pathMappings[0].Value != "$UserId" { + t.Errorf("got Value=%q, want $UserId", pathMappings[0].Value) + } + + if len(queryMappings) != 1 { + t.Fatalf("expected 1 query mapping, got %d", len(queryMappings)) + } + if queryMappings[0].Parameter != "Test.MyAPI.GetUser.status" { + t.Errorf("got Parameter=%q, want Test.MyAPI.GetUser.status", queryMappings[0].Parameter) + } + if queryMappings[0].Value != "'active'" { + t.Errorf("got Value=%q, want 'active'", queryMappings[0].Value) + } + if queryMappings[0].Included != "Yes" { + t.Errorf("got Included=%q, want Yes", queryMappings[0].Included) + } +} + +// Test: no operation info available (nil op) → all params go to QueryParameterMappings. +func TestBuildRestParameterMappings_NilOp_FallbackToQuery(t *testing.T) { + params := []ast.SendRestParamDef{ + {Name: "name", Expression: "$Name"}, + {Name: "email", Expression: "$Email"}, + } + pathMappings, queryMappings := buildRestParameterMappings(params, nil, "Test.MyAPI.PostData") + + if len(pathMappings) != 0 { + t.Errorf("expected 0 path mappings with nil op, got %d", len(pathMappings)) + } + if len(queryMappings) != 2 { + t.Errorf("expected 2 query mappings with nil op fallback, got %d", len(queryMappings)) + } +} + +// --- shouldSetBodyVariable tests --- + +// Test: JSON body → should NOT set BodyVariable. +func TestShouldSetBodyVariable_JsonBody_False(t *testing.T) { + op := &model.RestClientOperation{BodyType: "JSON"} + if shouldSetBodyVariable(op) { + t.Error("expected shouldSetBodyVariable=false for JSON body, got true") + } +} + +// Test: TEMPLATE/STRING body → should NOT set BodyVariable. +func TestShouldSetBodyVariable_TemplateBody_False(t *testing.T) { + op := &model.RestClientOperation{BodyType: "TEMPLATE"} + if shouldSetBodyVariable(op) { + t.Error("expected shouldSetBodyVariable=false for TEMPLATE body, got true") + } +} + +// Test: EXPORT_MAPPING body → should set BodyVariable. +func TestShouldSetBodyVariable_ExportMappingBody_True(t *testing.T) { + op := &model.RestClientOperation{BodyType: "EXPORT_MAPPING"} + if !shouldSetBodyVariable(op) { + t.Error("expected shouldSetBodyVariable=true for EXPORT_MAPPING body, got false") + } +} + +// Test: nil op (operation not found) → should set BodyVariable (preserve old behavior). +func TestShouldSetBodyVariable_NilOp_True(t *testing.T) { + if !shouldSetBodyVariable(nil) { + t.Error("expected shouldSetBodyVariable=true for nil op (fallback), got false") + } +} + +// Test: empty BodyType (no body) → should NOT set BodyVariable. +func TestShouldSetBodyVariable_NoBody_False(t *testing.T) { + op := &model.RestClientOperation{BodyType: ""} + if shouldSetBodyVariable(op) { + t.Error("expected shouldSetBodyVariable=false for empty BodyType, got true") + } +} + +// --- addSendRestRequestAction integration (via flowBuilder) --- + +// Test: SEND REST REQUEST with JSON body — BodyVariable must be nil. +func TestAddSendRestRequest_JsonBody_NoBodyVariable(t *testing.T) { + op := &model.RestClientOperation{ + Name: "PostJsonTemplate", + BodyType: "JSON", + Parameters: []*model.RestClientParameter{ + {Name: "Name", DataType: "String"}, + {Name: "Email", DataType: "String"}, + }, + } + svc := &model.ConsumedRestService{ + BaseElement: model.BaseElement{ID: "svc-1"}, + Name: "RC_TestApi", + Operations: []*model.RestClientOperation{op}, + } + + fb := &flowBuilder{ + objects: nil, + flows: nil, + posX: 100, + posY: 100, + spacing: 200, + varTypes: map[string]string{}, + declaredVars: map[string]string{}, + restServices: []*model.ConsumedRestService{svc}, + } + + stmt := &ast.SendRestRequestStmt{ + Operation: ast.QualifiedName{Module: "MfTest", Name: "RC_TestApi.PostJsonTemplate"}, + Parameters: []ast.SendRestParamDef{ + {Name: "Name", Expression: "$Name"}, + {Name: "Email", Expression: "$Email"}, + }, + BodyVariable: "JsonBody", + } + + fb.addSendRestRequestAction(stmt) + + if len(fb.objects) == 0 { + t.Fatal("expected at least one object in flowBuilder after addSendRestRequestAction") + } + + activity, ok := fb.objects[0].(*microflows.ActionActivity) + if !ok { + t.Fatalf("expected ActionActivity, got %T", fb.objects[0]) + } + + action, ok := activity.Action.(*microflows.RestOperationCallAction) + if !ok { + t.Fatalf("expected RestOperationCallAction, got %T", activity.Action) + } + + // For JSON body, BodyVariable must be nil + if action.BodyVariable != nil { + t.Errorf("expected BodyVariable=nil for JSON body, got %+v", action.BodyVariable) + } + + // Both params should be classified as path params (both are in op.Parameters) + if len(action.ParameterMappings) != 2 { + t.Errorf("expected 2 path parameter mappings, got %d", len(action.ParameterMappings)) + } + if len(action.QueryParameterMappings) != 0 { + t.Errorf("expected 0 query parameter mappings, got %d", len(action.QueryParameterMappings)) + } +} + +// Test: SEND REST REQUEST with EXPORT_MAPPING body — BodyVariable must be set. +func TestAddSendRestRequest_ExportMappingBody_HasBodyVariable(t *testing.T) { + op := &model.RestClientOperation{ + Name: "PostEntity", + BodyType: "EXPORT_MAPPING", + } + svc := &model.ConsumedRestService{ + BaseElement: model.BaseElement{ID: "svc-2"}, + Name: "EntityAPI", + Operations: []*model.RestClientOperation{op}, + } + + fb := &flowBuilder{ + objects: nil, + flows: nil, + posX: 100, + posY: 100, + spacing: 200, + varTypes: map[string]string{}, + declaredVars: map[string]string{}, + restServices: []*model.ConsumedRestService{svc}, + } + + stmt := &ast.SendRestRequestStmt{ + Operation: ast.QualifiedName{Module: "MfTest", Name: "EntityAPI.PostEntity"}, + BodyVariable: "MyEntity", + } + + fb.addSendRestRequestAction(stmt) + + activity, ok := fb.objects[0].(*microflows.ActionActivity) + if !ok { + t.Fatalf("expected ActionActivity, got %T", fb.objects[0]) + } + action, ok := activity.Action.(*microflows.RestOperationCallAction) + if !ok { + t.Fatalf("expected RestOperationCallAction, got %T", activity.Action) + } + + if action.BodyVariable == nil { + t.Error("expected BodyVariable to be set for EXPORT_MAPPING body, got nil") + } else if action.BodyVariable.VariableName != "MyEntity" { + t.Errorf("got BodyVariable.VariableName=%q, want MyEntity", action.BodyVariable.VariableName) + } +} diff --git a/mdl/executor/cmd_microflows_create.go b/mdl/executor/cmd_microflows_create.go index 3a61dc01..ac4ba445 100644 --- a/mdl/executor/cmd_microflows_create.go +++ b/mdl/executor/cmd_microflows_create.go @@ -22,6 +22,15 @@ func isBuiltinModuleEntity(moduleName string) bool { } // execCreateMicroflow handles CREATE MICROFLOW statements. +// loadRestServices returns all consumed REST services, or nil if no reader. +func (e *Executor) loadRestServices() ([]*model.ConsumedRestService, error) { + if e.reader == nil { + return nil, nil + } + svcs, err := e.reader.ListConsumedRestServices() + return svcs, err +} + func (e *Executor) execCreateMicroflow(s *ast.CreateMicroflowStmt) error { if e.writer == nil { return fmt.Errorf("not connected to a project") @@ -192,6 +201,8 @@ func (e *Executor) execCreateMicroflow(s *ast.CreateMicroflowStmt) error { // Get hierarchy for resolving page/microflow references hierarchy, _ := e.getHierarchy() + restServices, _ := e.loadRestServices() + builder := &flowBuilder{ posX: 200, posY: 200, @@ -202,6 +213,7 @@ func (e *Executor) execCreateMicroflow(s *ast.CreateMicroflowStmt) error { measurer: &layoutMeasurer{varTypes: varTypes}, reader: e.reader, hierarchy: hierarchy, + restServices: restServices, } mf.ObjectCollection = builder.buildFlowGraph(s.Body, s.ReturnType) From 8b750ac1eb3e9d5f59257b1a9a294a62a9eddcdc Mon Sep 17 00:00:00 2001 From: Ylber Date: Fri, 17 Apr 2026 10:00:53 +0200 Subject: [PATCH 3/3] docs: add mxcli-dev contributor command namespace and /mxcli-dev:review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Introduces a clear separation between commands meant for mxcli users (mendix/) and commands for contributors to this repo (mxcli-dev/). - Add .claude/commands/mxcli-dev/review.md — /mxcli-dev:review command with structured PR review workflow and a self-improving recurring findings table seeded from ako's reviews of #216 and #217 - Document the mendix: vs mxcli-dev: namespace split in CLAUDE.md so contributors know where to add new commands and why mxcli-dev/ is excluded from mxcli init sync Co-Authored-By: Claude Sonnet 4.6 --- .claude/commands/mxcli-dev/review.md | 42 ++++++++++++++++++++++++++++ CLAUDE.md | 13 ++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) create mode 100644 .claude/commands/mxcli-dev/review.md diff --git a/.claude/commands/mxcli-dev/review.md b/.claude/commands/mxcli-dev/review.md new file mode 100644 index 00000000..a4d4d15c --- /dev/null +++ b/.claude/commands/mxcli-dev/review.md @@ -0,0 +1,42 @@ +# /mxcli-dev:review — PR Review + +Run a structured review of the current branch's changes against the CLAUDE.md +checklist, then check the recurring findings table below for patterns that have +burned us before. + +## Steps + +1. Run `gh pr view` and `gh pr diff` (or `git diff main...HEAD`) to read the change. +2. Work through the CLAUDE.md "PR / Commit Review Checklist" in full. +3. Then check every row in the Recurring Findings table below — flag any match. +4. Report: blockers first, then moderate issues, then minor. Include a concrete fix + option for every blocker (not just "this is wrong"). +5. After the review: **add a row** to the Recurring Findings table for any new + pattern not already covered. + +--- + +## Recurring Findings + +Patterns caught in real reviews. Each row is a class of mistake worth checking +proactively. Add a row after every review that surfaces something new. + +| # | Finding | Category | Canonical fix | +|---|---------|----------|---------------| +| 1 | Formatter emits a keyword not present in `MDLParser.g4` → DESCRIBE output won't re-parse (e.g. `RANGE(...)`) | DESCRIBE roundtrip | Grep grammar before assuming a keyword is valid; if construct can't be expressed yet, emit `-- TypeName(field=value) — not yet expressible in MDL` | +| 2 | Output uses `$currentObject/Attr` prefix — non-idiomatic; Studio Pro uses bare attribute names | Idiomatic output | Verify against a real Studio Pro BSON sample before choosing a prefix convention | +| 3 | Malformed BSON field (missing key, wrong type) produces silent garbage output (e.g. `RANGE($x, , )`) | Error handling | Default missing numeric fields to `"0"`; or emit `-- malformed ` rather than broken MDL | +| 4 | No DESCRIBE roundtrip test — grammar gap went undetected until human review | Test coverage | Add roundtrip test: format struct → MDL string → parse → confirm no error | +| 5 | Hardcoded personal path in committed file (e.g. `/c/Users/Ylber.Sadiku/...`) | Docs quality | Use bare commands (`go test ./...`) without absolute paths in any committed doc or skill | +| 6 | Docs-only PR cites an unmerged PR as a "model example" — cited PR had blockers | Docs quality | Only cite merged, verified PRs; or annotate with known gaps if citing in-flight work | +| 7 | Skill/doc table references a function that doesn't exist (e.g. `formatActionStatement()` vs `formatAction()`) | Docs quality | Grep function names before writing: `grep -r "func formatA" mdl/executor/` | +| 8 | "Always X" rule is too absolute for trivial edge cases (e.g. "always write failing test first" for one-char typos) | Docs quality | Soften to "prefer X" or add an exception clause; include the reasoning so readers can judge edge cases | + +--- + +## After Every Review + +- [ ] All blockers have a concrete fix option stated. +- [ ] Recurring Findings table updated with any new pattern. +- [ ] If docs-only PR: every function name, path, and PR reference verified against + live code before approving. diff --git a/CLAUDE.md b/CLAUDE.md index d1c19dcc..5f6f9e5e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -360,11 +360,22 @@ mxcli new MyApp --version 10.24.0 --output-dir ./projects/my-app Steps performed: downloads MxBuild → `mx create-project` → `mxcli init` → downloads correct Linux mxcli binary for devcontainer. The result is a ready-to-open project with `.devcontainer/`, AI tooling, and a working `./mxcli` binary. +### Slash Command Namespaces + +Commands in `.claude/commands/` are organised by audience: + +| Namespace | Folder | Invoked as | Purpose | +|-----------|--------|------------|---------| +| `mendix:` | `.claude/commands/mendix/` | `/mendix:lint` | mxcli **user** commands — synced to Mendix projects via `mxcli init` | +| `mxcli-dev:` | `.claude/commands/mxcli-dev/` | `/mxcli-dev:review` | **Contributor** commands — this repo only, never synced to user projects | + +Both namespaces are discoverable by typing `/mxcli` in Claude Code. Add new contributor tooling (review workflows, debugging helpers, etc.) under `mxcli-dev/`. Add commands intended for Mendix project users under `mendix/`. + ### mxcli init `mxcli init` creates a `.claude/` folder with skills, commands, CLAUDE.md, and VS Code MDL extension in a target Mendix project. Source of truth for synced assets: - Skills: `reference/mendix-repl/templates/.claude/skills/` -- Commands: `.claude/commands/mendix/` +- Commands: `.claude/commands/mendix/` (the `mxcli-dev/` folder is **not** synced) - VS Code extension: `vscode-mdl/vscode-mdl-*.vsix` Build-time sync: `make build` syncs everything automatically. Individual targets: `make sync-skills`, `make sync-commands`, `make sync-vsix`.